Add tests

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
hyperledger · Feb 9, 2024 · 7220c48 · 7220c48
1 parent 43ee50e
commit 7220c48
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 81 deletions.
diff --git a/.../core/src/test/java/org/hyperledger/besu/ethereum/trie/bonsai/AbstractIsolationTests.java b/.../core/src/test/java/org/hyperledger/besu/ethereum/trie/bonsai/AbstractIsolationTests.java
@@ -187,7 +187,6 @@ protected StorageProvider createKeyValueStorageProvider() {
                         8388608 /*CACHE_CAPACITY*/,
                         false),
                 Arrays.asList(KeyValueSegmentIdentifier.values()),
-                DataStorageFormat.BONSAI,
                 RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS))
         .withCommonConfiguration(
             new BesuConfiguration() {

diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle
@@ -69,7 +69,7 @@ Calculated : ${currentHash}
 tasks.register('checkAPIChanges', FileStateChecker) {
   description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
   files = sourceSets.main.allJava.files
-  knownHash = '3+WNtdl1idY70N/MwVBbopU2ZWyWiu12YV1qaYXprZ8='
+  knownHash = 'zb0vFXnyTWFweXeIkzGoKBDcZ5HfYto12wmbmK0Px8Y='
 }
 check.dependsOn('checkAPIChanges')
 

diff --git a/...a/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java b/...a/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java
@@ -20,7 +20,6 @@
 import org.hyperledger.besu.plugin.services.BesuConfiguration;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 import org.hyperledger.besu.plugin.services.exception.StorageException;
-import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
 import org.hyperledger.besu.plugin.services.storage.KeyValueStorage;
 import org.hyperledger.besu.plugin.services.storage.KeyValueStorageFactory;
 import org.hyperledger.besu.plugin.services.storage.SegmentIdentifier;
@@ -55,7 +54,6 @@
 public class RocksDBKeyValueStorageFactory implements KeyValueStorageFactory {
 
   private static final Logger LOG = LoggerFactory.getLogger(RocksDBKeyValueStorageFactory.class);
-  private static final VersionedStorageFormat DEFAULT_VERSIONED_FORMAT = FOREST_WITH_VARIABLES;
   private static final EnumSet<BaseVersionedStorageFormat> SUPPORTED_VERSIONED_FORMATS =
       EnumSet.of(FOREST_WITH_VARIABLES, BONSAI_WITH_VARIABLES);
   private static final String NAME = "rocksdb";
@@ -74,76 +72,31 @@ public class RocksDBKeyValueStorageFactory implements KeyValueStorageFactory {
    * @param configuration the configuration
    * @param configuredSegments the segments
    * @param ignorableSegments the ignorable segments
-   * @param format the storage format
    * @param rocksDBMetricsFactory the rocks db metrics factory
    */
   public RocksDBKeyValueStorageFactory(
       final Supplier<RocksDBFactoryConfiguration> configuration,
       final List<SegmentIdentifier> configuredSegments,
       final List<SegmentIdentifier> ignorableSegments,
-      final DataStorageFormat format,
       final RocksDBMetricsFactory rocksDBMetricsFactory) {
     this.configuration = configuration;
     this.configuredSegments = configuredSegments;
     this.ignorableSegments = ignorableSegments;
     this.rocksDBMetricsFactory = rocksDBMetricsFactory;
-    this.databaseMetadata = DatabaseMetadata.defaultForNewDb(format);
   }
 
   /**
    * Instantiates a new RocksDb key value storage factory.
    *
    * @param configuration the configuration
    * @param configuredSegments the segments
-   * @param format the storage format
    * @param rocksDBMetricsFactory the rocks db metrics factory
    */
   public RocksDBKeyValueStorageFactory(
       final Supplier<RocksDBFactoryConfiguration> configuration,
       final List<SegmentIdentifier> configuredSegments,
-      final DataStorageFormat format,
       final RocksDBMetricsFactory rocksDBMetricsFactory) {
-    this(configuration, configuredSegments, List.of(), format, rocksDBMetricsFactory);
-  }
-
-  /**
-   * Instantiates a new Rocks db key value storage factory.
-   *
-   * @param configuration the configuration
-   * @param configuredSegments the segments
-   * @param ignorableSegments the ignorable segments
-   * @param rocksDBMetricsFactory the rocks db metrics factory
-   */
-  public RocksDBKeyValueStorageFactory(
-      final Supplier<RocksDBFactoryConfiguration> configuration,
-      final List<SegmentIdentifier> configuredSegments,
-      final List<SegmentIdentifier> ignorableSegments,
-      final RocksDBMetricsFactory rocksDBMetricsFactory) {
-    this(
-        configuration,
-        configuredSegments,
-        ignorableSegments,
-        DEFAULT_VERSIONED_FORMAT.getFormat(),
-        rocksDBMetricsFactory);
-  }
-
-  /**
-   * Instantiates a new Rocks db key value storage factory.
-   *
-   * @param configuration the configuration
-   * @param configuredSegments the segments
-   * @param rocksDBMetricsFactory the rocks db metrics factory
-   */
-  public RocksDBKeyValueStorageFactory(
-      final Supplier<RocksDBFactoryConfiguration> configuration,
-      final List<SegmentIdentifier> configuredSegments,
-      final RocksDBMetricsFactory rocksDBMetricsFactory) {
-    this(
-        configuration,
-        configuredSegments,
-        List.of(),
-        DEFAULT_VERSIONED_FORMAT.getFormat(),
-        rocksDBMetricsFactory);
+    this(configuration, configuredSegments, List.of(), rocksDBMetricsFactory);
   }
 
   @Override

diff --git a/...g/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java b/...g/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java
@@ -73,13 +73,6 @@ public void shouldCreateCorrectMetadataFileForLatestVersionForNewDb() throws Exc
     }
   }
 
-  private void mockCommonConfiguration(
-      final Path tempDataDir, final Path tempDatabaseDir, final DataStorageFormat format) {
-    when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir);
-    when(commonConfiguration.getDataPath()).thenReturn(tempDataDir);
-    lenient().when(commonConfiguration.getDatabaseFormat()).thenReturn(format);
-  }
-
   @Test
   public void shouldFailIfDbExistsAndNoMetadataFileFound() throws Exception {
     final Path tempDataDir = temporaryFolder.resolve("data");
@@ -122,7 +115,7 @@ public void shouldDetectCorrectMetadataV1() throws Exception {
   }
 
   @Test
-  public void shouldDetectCorrectVersionInCaseOfRollback() throws Exception {
+  public void shouldFailInCaseOfUnmanagedRollback() throws Exception {
     final Path tempDataDir = temporaryFolder.resolve("data");
     final Path tempDatabaseDir = temporaryFolder.resolve("db");
     Files.createDirectories(tempDatabaseDir);
@@ -133,22 +126,20 @@ public void shouldDetectCorrectVersionInCaseOfRollback() throws Exception {
 
     final RocksDBKeyValueStorageFactory storageFactory =
         new RocksDBKeyValueStorageFactory(
-            () -> rocksDbConfiguration,
-            segments,
-            BONSAI,
-            RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS);
+            () -> rocksDbConfiguration, segments, RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS);
 
     storageFactory.create(segment, commonConfiguration, metricsSystem);
     storageFactory.close();
 
+    Utils.createDatabaseMetadataV2(tempDataDir, BONSAI, 1);
+
     final RocksDBKeyValueStorageFactory rolledbackStorageFactory =
         new RocksDBKeyValueStorageFactory(
-            () -> rocksDbConfiguration,
-            segments,
-            FOREST,
-            RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS);
-    rolledbackStorageFactory.create(segment, commonConfiguration, metricsSystem);
-    rolledbackStorageFactory.close();
+            () -> rocksDbConfiguration, segments, RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS);
+    assertThatThrownBy(
+            () -> rolledbackStorageFactory.create(segment, commonConfiguration, metricsSystem))
+        .isInstanceOf(StorageException.class)
+        .hasMessageStartingWith("Database unsafe downgrade detect");
   }
 
   @Test
@@ -167,12 +158,12 @@ public void shouldThrowExceptionWhenVersionNumberIsInvalid() throws Exception {
                         segments,
                         RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS)
                     .create(segment, commonConfiguration, metricsSystem))
-        .isInstanceOf(StorageException.class);
+        .isInstanceOf(StorageException.class)
+        .hasMessageStartingWith("Unsupported db version");
   }
 
   @Test
-  public void shouldThrowExceptionWhenExistingDatabaseVersionDifferentFromConfig()
-      throws Exception {
+  public void shouldThrowExceptionWhenExistingDatabaseFormatDiffersFromConfig() throws Exception {
 
     final DataStorageFormat actualDatabaseFormat = FOREST;
     final DataStorageFormat expectedDatabaseFormat = BONSAI;
@@ -185,12 +176,6 @@ public void shouldThrowExceptionWhenExistingDatabaseVersionDifferentFromConfig()
 
     Utils.createDatabaseMetadataV2(tempDataDir, FOREST, 2);
 
-    String exceptionMessage =
-        String.format(
-            "Database format mismatch: DB at %s is %s but config expects %s. "
-                + "Please check your config.",
-            tempDataDir.toAbsolutePath(), actualDatabaseFormat, expectedDatabaseFormat);
-
     assertThatThrownBy(
             () ->
                 new RocksDBKeyValueStorageFactory(
@@ -199,7 +184,10 @@ public void shouldThrowExceptionWhenExistingDatabaseVersionDifferentFromConfig()
                         RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS)
                     .create(segment, commonConfiguration, metricsSystem))
         .isInstanceOf(StorageException.class)
-        .hasMessage(exceptionMessage);
+        .hasMessage(
+            "Database format mismatch: DB at %s is %s but config expects %s. "
+                + "Please check your config.",
+            tempDataDir.toAbsolutePath(), actualDatabaseFormat, expectedDatabaseFormat);
   }
 
   @Test
@@ -240,9 +228,11 @@ public void shouldThrowExceptionWhenMetaDataFileIsCorrupted() throws Exception {
                         segments,
                         RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS)
                     .create(segment, commonConfiguration, metricsSystem))
-        .isInstanceOf(IllegalStateException.class);
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessageStartingWith("Invalid metadata file");
+    ;
 
-    Utils.createDatabaseMetadataRaw(tempDataDir, "{\"version\":\"iomedae\"}");
+    Utils.createDatabaseMetadataRaw(tempDataDir, "{\"version\"=1}");
 
     assertThatThrownBy(
             () ->
@@ -251,7 +241,9 @@ public void shouldThrowExceptionWhenMetaDataFileIsCorrupted() throws Exception {
                         segments,
                         RocksDBMetricsFactory.PUBLIC_ROCKS_DB_METRICS)
                     .create(segment, commonConfiguration, metricsSystem))
-        .isInstanceOf(IllegalStateException.class);
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessageStartingWith("Invalid metadata file");
+    ;
   }
 
   @Test
@@ -276,4 +268,11 @@ public void shouldCreateDBCorrectlyIfSymlink() throws Exception {
           .isEqualTo(BaseVersionedStorageFormat.FOREST_WITH_VARIABLES);
     }
   }
+
+  private void mockCommonConfiguration(
+      final Path tempDataDir, final Path tempDatabaseDir, final DataStorageFormat format) {
+    when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir);
+    when(commonConfiguration.getDataPath()).thenReturn(tempDataDir);
+    lenient().when(commonConfiguration.getDatabaseFormat()).thenReturn(format);
+  }
 }