From a54fa3d78b427d7b8278442acbe86ebebcf6c2e7 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Tue, 27 Jan 2015 11:31:59 +0000
Subject: [PATCH] OPENDJ-1716 Various PluggableBackend/Storage refactorings Code review: Nicolas Capponi

---
 opendj3-server-dev/src/server/org/opends/server/backends/persistit/PersistItStorage.java     |   27 +++++++-
 opendj3-server-dev/build.xml                                                                 |    4 -
 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteableStorage.java |    4 
 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteOperation.java   |   15 ++++
 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Importer.java         |   25 +++++++-
 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Storage.java          |   67 +++++++++++++++++++++-
 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/ReadOperation.java    |   19 +++++
 7 files changed, 141 insertions(+), 20 deletions(-)

diff --git a/opendj3-server-dev/build.xml b/opendj3-server-dev/build.xml
index 0ab7ab5..c8e48bf 100644
--- a/opendj3-server-dev/build.xml
+++ b/opendj3-server-dev/build.xml
@@ -22,7 +22,7 @@
  !
  !
  !      Copyright 2006-2010 Sun Microsystems, Inc.
- !      Portions Copyright 2011-2014 ForgeRock AS
+ !      Portions Copyright 2011-2015 ForgeRock AS
  !      Portions Copyright 2012 Delta Victor Consultants
  ! -->
 
@@ -43,7 +43,6 @@
   <!-- General server-wide properties                                 -->
   <property name="src.dir"          location="src/server"              />
   <property name="pluggablebackend.pkg" value="org/opends/server/backends/pluggable" />
-  <property name="persistit.pkg"        value="org/opends/server/backends/persistit" />
   <property name="build.dir"        location="build"                   />
   <property name="classes.dir"      location="${build.dir}/classes"    />
   <property name="build.lib.dir"    location="${build.dir}/lib"        />
@@ -681,7 +680,6 @@
         <include name="**/*.java"/>
         <exclude name="**/PublicAPI.java" />
         <exclude name="${pluggablebackend.pkg}/**/*.java" />
-        <exclude name="${persistit.pkg}/*.java" />
       </fileset>
       <formatter type="plain" />
     </checkstyle>
diff --git a/opendj3-server-dev/src/server/org/opends/server/backends/persistit/PersistItStorage.java b/opendj3-server-dev/src/server/org/opends/server/backends/persistit/PersistItStorage.java
index c4d9dae..df2b606 100644
--- a/opendj3-server-dev/src/server/org/opends/server/backends/persistit/PersistItStorage.java
+++ b/opendj3-server-dev/src/server/org/opends/server/backends/persistit/PersistItStorage.java
@@ -39,6 +39,7 @@
 import org.forgerock.opendj.ldap.ByteSequence;
 import org.forgerock.opendj.ldap.ByteString;
 import org.opends.server.admin.std.server.PersistitBackendCfg;
+import org.opends.server.admin.std.server.PluggableBackendCfg;
 import org.opends.server.backends.pluggable.spi.Cursor;
 import org.opends.server.backends.pluggable.spi.Importer;
 import org.opends.server.backends.pluggable.spi.ReadOperation;
@@ -63,13 +64,14 @@
 import com.persistit.exception.PersistitException;
 import com.persistit.exception.RollbackException;
 
-@SuppressWarnings("javadoc")
+/** PersistIt database implementation of the {@link Storage} engine. */
 public final class PersistItStorage implements Storage
 {
   private static final String VOLUME_NAME = "dj";
   /** The buffer / page size used by the PersistIt storage. */
   private static final int BUFFER_SIZE = 16 * 1024;
 
+  /** PersistIt implementation of the {@link Cursor} interface. */
   private final class CursorImpl implements Cursor
   {
     private ByteString currentKey;
@@ -190,6 +192,7 @@
     }
   }
 
+  /** PersistIt implementation of the {@link Importer} interface. */
   private final class ImporterImpl implements Importer
   {
     private final TreeBuilder importer = new TreeBuilder(db);
@@ -245,6 +248,7 @@
     }
   }
 
+  /** PersistIt implementation of the {@link WriteableStorage} interface. */
   private final class StorageImpl implements WriteableStorage
   {
     private final Map<TreeName, Exchange> exchanges = new HashMap<TreeName, Exchange>();
@@ -484,6 +488,7 @@
   private Configuration dbCfg;
   private PersistitBackendCfg config;
 
+  /** {@inheritDoc} */
   @Override
   public void close()
   {
@@ -501,15 +506,18 @@
     }
   }
 
+  /** {@inheritDoc} */
   @Override
   public void closeTree(final TreeName treeName)
   {
     // nothing to do, in persistit you close the volume itself
   }
 
+  /** {@inheritDoc} */
   @Override
-  public void initialize(final PersistitBackendCfg cfg)
+  public void initialize(final PluggableBackendCfg configuration)
   {
+    final PersistitBackendCfg cfg = (PersistitBackendCfg) configuration;
     backendDirectory = new File(getFileForPath(cfg.getDBDirectory()), cfg.getBackendId());
     config = cfg;
     dbCfg = new Configuration();
@@ -520,7 +528,7 @@
         Long.MAX_VALUE / BUFFER_SIZE, 2048, true, false, false)));
     final BufferPoolConfiguration bufferPoolCfg = getBufferPoolCfg();
     bufferPoolCfg.setMaximumCount(Integer.MAX_VALUE);
-    if (cfg.getDBCacheSize() > 0l)
+    if (cfg.getDBCacheSize() > 0)
     {
       bufferPoolCfg.setMaximumMemory(cfg.getDBCacheSize());
     }
@@ -536,12 +544,14 @@
     return dbCfg.getBufferPoolMap().get(BUFFER_SIZE);
   }
 
+  /** {@inheritDoc} */
   @Override
   public boolean isValid()
   {
     return !db.isFatal();
   }
 
+  /** {@inheritDoc} */
   @Override
   public void open()
   {
@@ -563,6 +573,7 @@
     }
   }
 
+  /** {@inheritDoc} */
   @Override
   public <T> T read(final ReadOperation<T> operation) throws Exception
   {
@@ -604,6 +615,7 @@
     }
   }
 
+  /** {@inheritDoc} */
   @Override
   public Importer startImport()
   {
@@ -614,12 +626,17 @@
 
   /**
    * Replace persistit reserved comma character with an underscore character.
+   *
+   * @param suffix
+   *          the suffix name to convert
+   * @return a new String suitable for use as a suffix name
    */
-  public String toSuffixName(final String prefix)
+  public String toSuffixName(final String suffix)
   {
-    return prefix.replaceAll("[,=]", "_");
+    return suffix.replaceAll("[,=]", "_");
   }
 
+  /** {@inheritDoc} */
   @Override
   public void write(final WriteOperation operation) throws Exception
   {
diff --git a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Importer.java b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Importer.java
index f2df235..a31a73a 100644
--- a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Importer.java
+++ b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Importer.java
@@ -21,21 +21,40 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2014 ForgeRock AS
+ *      Copyright 2014-2015 ForgeRock AS
  */
-
 package org.opends.server.backends.pluggable.spi;
 
 import java.io.Closeable;
 
 import org.forgerock.opendj.ldap.ByteSequence;
 
+/**
+ * Allows to run an import. For performance reasons, imports are run without transactions.
+ */
 public interface Importer extends Closeable
 {
+  /**
+   * Creates a new tree identified by the provided name.
+   *
+   * @param name
+   *          the tree name
+   */
   void createTree(TreeName name);
 
-  void put(TreeName name, ByteSequence key, ByteSequence value);
+  /**
+   * Creates a record with the provided key and value in the tree identified by the provided name.
+   *
+   * @param treeName
+   *          the tree name
+   * @param key
+   *          the new record's key
+   * @param value
+   *          the new record's value
+   */
+  void put(TreeName treeName, ByteSequence key, ByteSequence value);
 
+  /** {@inheritDoc} */
   @Override
   void close();
 }
\ No newline at end of file
diff --git a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/ReadOperation.java b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/ReadOperation.java
index 9d9fd6c..d401864 100644
--- a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/ReadOperation.java
+++ b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/ReadOperation.java
@@ -21,12 +21,27 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2014 ForgeRock AS
+ *      Copyright 2014-2015 ForgeRock AS
  */
-
 package org.opends.server.backends.pluggable.spi;
 
+/**
+ * Function performing a read operation.
+ *
+ * @param <T>
+ *          type of the value that is read and returned by this read operation
+ */
+// @FunctionalInterface
 public interface ReadOperation<T>
 {
+  /**
+   * Executes a read operation, and returns the read value.
+   *
+   * @param txn
+   *          the read transaction where to execute the read operation
+   * @return the read value
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   */
   T run(ReadableStorage txn) throws Exception;
 }
\ No newline at end of file
diff --git a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Storage.java b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Storage.java
index 48ef1ac..c31844e 100644
--- a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Storage.java
+++ b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/Storage.java
@@ -21,30 +21,91 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2014 ForgeRock AS
+ *      Copyright 2014-2015 ForgeRock AS
  */
 package org.opends.server.backends.pluggable.spi;
 
 import java.io.Closeable;
 
-import org.opends.server.admin.std.server.PersistitBackendCfg;
+import org.opends.server.admin.std.server.PluggableBackendCfg;
 
+/**
+ * This interface abstracts the underlying storage engine,
+ * isolating the pluggable backend generic code from a particular storage engine implementation.
+ */
 public interface Storage extends Closeable
 {
-  void initialize(PersistitBackendCfg cfg) throws Exception;
+  /**
+   * Initializes the storage engine before opening it.
+   *
+   * @param cfg
+   *          the configuration object
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   * @see #open() to open the storage engine
+   */
+  void initialize(PluggableBackendCfg cfg) throws Exception;
 
+  /**
+   * Starts the import operation.
+   *
+   * @return a new Importer object which must be closed to release all resources
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   * @see #close() to release all resources once import is finished
+   */
   Importer startImport() throws Exception;
 
+  /**
+   * Opens the storage engine to allow executing operations on it.
+   *
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   * @see #close() to release all resources once import is finished
+   */
   void open() throws Exception;
 
+  /**
+   * Executes a read operation. In case of a read operation rollback, implementations must ensure
+   * the read operation is retried until it succeeds.
+   *
+   * @param <T>
+   *          type of the value returned
+   * @param readOperation
+   *          the read operation to execute
+   * @return the value read by the read operation
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   */
   <T> T read(ReadOperation<T> readOperation) throws Exception;
 
+  /**
+   * Executes a write operation. In case of a write operation rollback, implementations must ensure
+   * the write operation is retried until it succeeds.
+   *
+   * @param writeOperation
+   *          the write operation to execute
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   */
   void write(WriteOperation writeOperation) throws Exception;
 
+  /**
+   * Closes the tree identified by the provided name.
+   *
+   * @param treeName
+   *          the tree name
+   */
   void closeTree(TreeName treeName);
 
+  /**
+   * Returns whether the storage engine is in a valid state, i.e. whether it can be used for processing.
+   *
+   * @return {@code true} if the storage engine is in a valid state, {@code false} otherwise
+   */
   boolean isValid();
 
+  /** {@inheritDoc} */
   @Override
   void close();
 }
\ No newline at end of file
diff --git a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteOperation.java b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteOperation.java
index e99b1a2..6ba146e 100644
--- a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteOperation.java
+++ b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteOperation.java
@@ -21,12 +21,23 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2014 ForgeRock AS
+ *      Copyright 2014-2015 ForgeRock AS
  */
-
 package org.opends.server.backends.pluggable.spi;
 
+/**
+ * Function performing a write operation.
+ */
+// @FunctionalInterface
 public interface WriteOperation
 {
+  /**
+   * Executes a write operation.
+   *
+   * @param txn
+   *          the write transaction where to execute the write operation
+   * @throws Exception
+   *           if a problem occurs with the underlying storage engine
+   */
   void run(WriteableStorage txn) throws Exception;
 }
\ No newline at end of file
diff --git a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteableStorage.java b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteableStorage.java
index 96c00ca..7d0f13f 100644
--- a/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteableStorage.java
+++ b/opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteableStorage.java
@@ -41,7 +41,7 @@
   void openTree(TreeName name);
 
   /**
-   * Truncates the tree having the provided name. It removes all the records in the tree. 
+   * Truncates the tree having the provided name. It removes all the records in the tree.
    *
    * @param name
    *          the tree name
@@ -82,7 +82,7 @@
   /**
    * Creates a new record with the provided key and value, in the tree whose name is provided, if
    * the key was not previously associated to any record.
-   * 
+   *
    * @param treeName
    *          the tree name
    * @param key

--
Gitblit v1.10.0