Persist changes to ProtectedStorageEntrys

With the addition of ProtectedStorageEntrys, there are now persistable
maps that have different payloads and the same keys. In the
ProtectedDataStoreService case, the value is the ProtectedStorageEntry
which has a createdTimeStamp, sequenceNumber, and signature that can
all change, but still contain an identical payload.

Previously, the service was only updating the on-disk representation on
the first object and never again. So, when it was recreated from disk it
would not have any of the updated metadata. This was just copied from the
append-only implementation where the value was the Payload
which was immutable.

This hasn't caused any issues to this point, but it causes strange behavior
such as always receiving seqNr==1 items from seednodes on startup. It
is good practice to keep the in-memory objects and on-disk objects in
sync and removes an unexpected failure in future dev work that expects
the same behavior as the append-only on-disk objects.
This commit is contained in:
Julian Knutsen 2019-11-19 18:27:08 -08:00
parent 66f71e59f8
commit f3faf4bb63
No known key found for this signature in database
GPG key ID: D85F536DB3615B2D
4 changed files with 9 additions and 12 deletions

View file

@ -428,9 +428,8 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
// Persist ProtectedStorageEntrys carrying PersistablePayload payloads and signal listeners on changes
if (protectedStoragePayload instanceof PersistablePayload) {
ProtectedStorageEntry previous = protectedDataStoreService.putIfAbsent(hashOfPayload, protectedStorageEntry);
if (previous == null)
protectedDataStoreListeners.forEach(e -> e.onAdded(protectedStorageEntry));
protectedDataStoreService.put(hashOfPayload, protectedStorageEntry);
protectedDataStoreListeners.forEach(e -> e.onAdded(protectedStorageEntry));
}
return true;

View file

@ -56,6 +56,11 @@ public abstract class MapStoreService<T extends PersistableEnvelope, R extends P
public abstract boolean canHandle(R payload);
void put(P2PDataStorage.ByteArray hash, R payload) {
getMap().put(hash, payload);
persist();
}
R putIfAbsent(P2PDataStorage.ByteArray hash, R payload) {
R previous = getMap().putIfAbsent(hash, payload);
persist();

View file

@ -65,7 +65,7 @@ public class ProtectedDataStoreService {
services.stream()
.filter(service -> service.canHandle(entry))
.forEach(service -> {
service.putIfAbsent(hash, entry);
service.put(hash, entry);
});
}

View file

@ -221,16 +221,9 @@ public class TestState {
if (expectedStateChange) {
Assert.assertEquals(protectedStorageEntry, this.mockedStorage.getMap().get(hashMapHash));
// PersistablePayload payloads need to be written to disk and listeners signaled... unless the hash already exists in the protectedDataStore.
// Note: this behavior is different from the HashMap listeners that are signaled on an increase in seq #, even if the hash already exists.
// TODO: Should the behavior be identical between this and the HashMap listeners?
// TODO: Do we want ot overwrite stale values in order to persist updated sequence numbers and timestamps?
if (protectedStorageEntry.getProtectedStoragePayload() instanceof PersistablePayload && beforeState.protectedStorageEntryBeforeOpDataStoreMap == null) {
if (protectedStorageEntry.getProtectedStoragePayload() instanceof PersistablePayload) {
Assert.assertEquals(protectedStorageEntry, this.protectedDataStoreService.getMap().get(hashMapHash));
verify(this.protectedDataStoreListener).onAdded(protectedStorageEntry);
} else {
Assert.assertEquals(beforeState.protectedStorageEntryBeforeOpDataStoreMap, this.protectedDataStoreService.getMap().get(hashMapHash));
verify(this.protectedDataStoreListener, never()).onAdded(protectedStorageEntry);
}
verify(this.hashMapChangedListener).onAdded(Collections.singletonList(protectedStorageEntry));