From 62d2f37a4e9ff4c637ff83e3d9dbbfd292552aaa Mon Sep 17 00:00:00 2001 From: Jakub Morvay Date: Mon, 15 May 2017 17:13:53 +0200 Subject: [PATCH] Bug 8444 - Persistent prefix-based shard cannot load its snapshot Since the name is URL-encoded, we have to make sure it does not get double-encoded -- hence we need to make a pass of URL-decoding before we use the result. Change-Id: I20fe8702ad7e405a8b68d8bda2f9ce4522f2dfd0 Signed-off-by: Jakub Morvay --- .../persistence/LocalSnapshotStore.java | 15 ++++++++++- .../persistence/LocalSnapshotStoreTest.java | 25 +++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java index 3e170fd326..d9b7adc3c9 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStore.java @@ -28,6 +28,7 @@ import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -281,7 +282,9 @@ public class LocalSnapshotStore extends SnapshotStore { } try { - String persistenceId = name.substring(PERSISTENCE_ID_START_INDEX, persistenceIdEndIndex); + // Since the persistenceId is url encoded in the filename, we need + // to decode relevant filename's part to obtain persistenceId back + String persistenceId = decode(name.substring(PERSISTENCE_ID_START_INDEX, persistenceIdEndIndex)); long sequenceNumber = Long.parseLong(name.substring(persistenceIdEndIndex + 1, sequenceNumberEndIndex)); long timestamp = Long.parseLong(name.substring(sequenceNumberEndIndex + 1)); return new SnapshotMetadata(persistenceId, sequenceNumber, timestamp); @@ -312,6 +315,16 @@ public class LocalSnapshotStore extends SnapshotStore { } } + private static String decode(final String str) { + try { + return URLDecoder.decode(str, StandardCharsets.UTF_8.name()); + } catch (final UnsupportedEncodingException e) { + // Shouldn't happen + LOG.warn("Error decoding {}", str, e); + return str; + } + } + @VisibleForTesting static int compare(final SnapshotMetadata m1, final SnapshotMetadata m2) { return (int) (!m1.persistenceId().equals(m2.persistenceId()) diff --git a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStoreTest.java b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStoreTest.java index 964b663d6f..b69b6345cc 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStoreTest.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/persistence/LocalSnapshotStoreTest.java @@ -33,6 +33,9 @@ import com.typesafe.config.ConfigFactory; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.SerializationUtils; import org.junit.After; @@ -51,6 +54,7 @@ import scala.Option; */ public class LocalSnapshotStoreTest { private static final String PERSISTENCE_ID = "member-1-shard-default-config"; + private static final String PREFIX_BASED_SHARD_PERSISTENCE_ID = "member-1-shard-id-ints!-config"; private static ActorSystem system; private static ActorRef snapshotStore; @@ -86,6 +90,10 @@ public class LocalSnapshotStoreTest { createSnapshotFile(PERSISTENCE_ID, "two", 1, 2000); createSnapshotFile(PERSISTENCE_ID, "three", 1, 3000); + createSnapshotFile(PREFIX_BASED_SHARD_PERSISTENCE_ID, "foo", 0, 1000); + createSnapshotFile(PREFIX_BASED_SHARD_PERSISTENCE_ID, "bar", 1, 2000); + createSnapshotFile(PREFIX_BASED_SHARD_PERSISTENCE_ID, "foobar", 1, 3000); + createSnapshotFile("member-1-shard-default-oper", "foo", 0, 1000); createSnapshotFile("member-1-shard-toaster-oper", "foo", 0, 1000); new File(SNAPSHOT_DIR, "other").createNewFile(); @@ -102,6 +110,17 @@ public class LocalSnapshotStoreTest { assertEquals("SelectedSnapshot present", TRUE, possibleSnapshot.nonEmpty()); assertEquals("SelectedSnapshot metadata", metadata3, possibleSnapshot.get().metadata()); assertEquals("SelectedSnapshot snapshot", "three", possibleSnapshot.get().snapshot()); + + snapshotStore.tell(new LoadSnapshot(PREFIX_BASED_SHARD_PERSISTENCE_ID, + SnapshotSelectionCriteria.latest(), Long.MAX_VALUE), probe.getRef()); + result = probe.expectMsgClass(LoadSnapshotResult.class); + possibleSnapshot = result.snapshot(); + + SnapshotMetadata prefixBasedShardMetada3 = new SnapshotMetadata(PREFIX_BASED_SHARD_PERSISTENCE_ID, 1, 3000); + + assertEquals("SelectedSnapshot present", TRUE, possibleSnapshot.nonEmpty()); + assertEquals("SelectedSnapshot metadata", prefixBasedShardMetada3, possibleSnapshot.get().metadata()); + assertEquals("SelectedSnapshot snapshot", "foobar", possibleSnapshot.get().snapshot()); } @Test @@ -175,7 +194,9 @@ public class LocalSnapshotStoreTest { } } - private static String toSnapshotName(String persistenceId, int seqNr, int timestamp) { - return "snapshot-" + persistenceId + "-" + seqNr + "-" + timestamp; + private static String toSnapshotName(String persistenceId, int seqNr, int timestamp) + throws UnsupportedEncodingException { + final String encodedPersistenceId = URLEncoder.encode(persistenceId, StandardCharsets.UTF_8.name()); + return "snapshot-" + encodedPersistenceId + "-" + seqNr + "-" + timestamp; } } -- 2.36.6