OPENDJ-1716 Various PluggableBackend/Storage refactorings Code review: Nico...

Jean-Noel Rouvignac

27.27.2015 8b1a3529580def922b2188a998fdebb65c756de0

OPENDJ-1716 Various PluggableBackend/Storage refactorings
Code review: Nicolas Capponi

PersistItStorage.java:
Removed remove(), superseded by delete() to go along with CRUD metaphor.
Changed update() to only update if the new value differs from the old value + returned a boolean indicating if the record was updated in the tree.

ReadableStorage.java:
Added javadocs.

UpdateFunction.java:
In update(), now return a boolean.

WriteableStorage.java:
Removed remove(), superseded by delete() to go along with CRUD metaphor.
Added javadocs.

DatabaseContainer.java:
Consequence of replacing WriteableStorage delete() by remove().

 opendj3-server-dev/src/server/org/opends/server/backends/persistit/PersistItStorage.java

@@ -267,13 +267,13 @@
    }

    @Override
    public void delete(final TreeName treeName, final ByteSequence key)
    public boolean delete(final TreeName treeName, final ByteSequence key)
    {
      try
      {
        final Exchange ex = getExchangeFromCache(treeName);
        bytesToKey(ex.getKey(), key);
        ex.remove();
        return ex.remove();
      }
      catch (final PersistitException e)
      {
@@ -388,21 +388,6 @@
    }

    @Override
    public boolean remove(final TreeName treeName, final ByteSequence key)
    {
      try
      {
        final Exchange ex = getExchangeFromCache(treeName);
        bytesToKey(ex.getKey(), key);
        return ex.remove();
      }
      catch (final PersistitException e)
      {
        throw new StorageRuntimeException(e);
      }
    }

    @Override
    public void renameTree(final TreeName oldTreeName,
        final TreeName newTreeName)
    {
@@ -423,8 +408,7 @@
    }

    @Override
    public void update(final TreeName treeName, final ByteSequence key,
        final UpdateFunction f)
    public boolean update(final TreeName treeName, final ByteSequence key, final UpdateFunction f)
    {
      try
      {
@@ -433,8 +417,13 @@
        ex.fetch();
        final ByteSequence oldValue = valueToBytes(ex.getValue());
        final ByteSequence newValue = f.computeNewValue(oldValue);
        ex.getValue().clear().putByteArray(newValue.toByteArray());
        ex.store();
        if (!equals(newValue, oldValue))
        {
          ex.getValue().clear().putByteArray(newValue.toByteArray());
          ex.store();
          return true;
        }
        return false;
      }
      catch (final Exception e)
      {
@@ -442,6 +431,15 @@
      }
    }

    private boolean equals(ByteSequence b1, ByteSequence b2)
    {
      if (b1 == null)
      {
        return b2 == null;
      }
      return b1.equals(b2);
    }

    private Exchange getExchangeFromCache(final TreeName treeName)
        throws PersistitException
    {

 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/DatabaseContainer.java

@@ -179,7 +179,7 @@
   */
  boolean delete(WriteableStorage txn, ByteSequence key) throws StorageRuntimeException
  {
    boolean result = txn.remove(treeName, key);
    final boolean result = txn.delete(treeName, key);
    if (logger.isTraceEnabled())
    {
      logger.trace(messageToLog(result, treeName, txn, key, null));

 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/ReadableStorage.java

@@ -21,21 +21,49 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2014 ForgeRock AS
 *      Copyright 2014-2015 ForgeRock AS
 */

package org.opends.server.backends.pluggable.spi;

import org.forgerock.opendj.ldap.ByteSequence;
import org.forgerock.opendj.ldap.ByteString;

/**
 * Represents a readable transaction on a storage engine.
 */
public interface ReadableStorage
{
  ByteString read(TreeName name, ByteSequence key);
  /**
   * Reads the record's value associated to the provided key, in the tree whose name is provided.
   *
   * @param treeName
   *          the tree name
   * @param key
   *          the record's key
   * @return the record's value, or {@code null} if none exists
   */
  ByteString read(TreeName treeName, ByteSequence key);

  ByteString getRMW(TreeName name, ByteSequence key);
  /**
   * Reads the record's value associated to the provided key in a Read-Modify-Write fashion, in the
   * tree whose name is provided.
   *
   * @param treeName
   *          the tree name
   * @param key
   *          the record's key
   * @return the record's value, or {@code null} if none exists
   * @deprecated use {@link #update(TreeName, ByteSequence, UpdateFunction)} instead
   */
  @Deprecated
  ByteString getRMW(TreeName treeName, ByteSequence key);

  Cursor openCursor(TreeName name);

  // TODO: contains, etc.
  /**
   * Opens a cursor on the tree whose name is provided.
   *
   * @param treeName
   *          the tree name
   * @return a new cursor
   */
  Cursor openCursor(TreeName treeName);
}

 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/UpdateFunction.java

@@ -21,12 +21,16 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2014 ForgeRock AS
 *      Copyright 2014-2015 ForgeRock AS
 */
package org.opends.server.backends.pluggable.spi;

import org.forgerock.opendj.ldap.ByteSequence;

/**
 * Function that computes the new value of a record for a Read-Modify-Write operation inside a transaction.
 */
// @FunctionalInterface
public interface UpdateFunction {
    /**
     * Computes the new value for a record based on the record's existing
@@ -35,8 +39,8 @@
     * @param oldValue
     *            The record's existing content, or {@code null} if the record
     *            does not exist at the moment and is about to be created.
     * @return The new value for the record, or {@code null} if the record
     *         should be completely removed.
     * @return The new value for the record (which may be {@code null} if the record should be removed),
     *         or the old value if no update is required.
     */
    ByteSequence computeNewValue(ByteSequence oldValue);
}

 opendj3-server-dev/src/server/org/opends/server/backends/pluggable/spi/WriteableStorage.java

@@ -21,29 +21,102 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2014 ForgeRock AS
 *      Copyright 2014-2015 ForgeRock AS
 */
package org.opends.server.backends.pluggable.spi;

import org.forgerock.opendj.ldap.ByteSequence;

/**
 * Represents a writeable transaction on a storage engine.
 */
public interface WriteableStorage extends ReadableStorage
{
  /**
   * Opens the tree having the provided name. The tree is created if does not already exist.
   *
   * @param name
   *          the tree name
   */
  void openTree(TreeName name);

  /**
   * Truncates the tree having the provided name. It removes all the records in the tree. 
   *
   * @param name
   *          the tree name
   */
  void truncateTree(TreeName name);

  /**
   * Renames the tree from the old to the new name.
   *
   * @param oldName
   *          the old tree name
   * @param newName
   *          the new tree name
   */
  void renameTree(TreeName oldName, TreeName newName);

  /**
   * Deletes the tree having the provided name.
   *
   * @param name
   *          the tree name
   */
  void deleteTree(TreeName name);

  void create(TreeName name, ByteSequence key, ByteSequence value);
  /**
   * Creates a new record with the provided key and value, in the tree whose name is provided.
   * If a previous record is associated to the provided key, then it will be replaced by the new record.
   *
   * @param treeName
   *          the tree name
   * @param key
   *          the key of the new record
   * @param value
   *          the value of the new record
   */
  void create(TreeName treeName, ByteSequence key, ByteSequence value);

  /**
   * 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
   *          the key of the new record
   * @param value
   *          the value of the new record
   * @return {@code true} if the new record could be created, {@code false} otherwise
   * @deprecated use {@link #update(TreeName, ByteSequence, UpdateFunction)} instead
   */
  @Deprecated
  boolean putIfAbsent(TreeName treeName, ByteSequence key, ByteSequence value);

  void update(TreeName treeName, ByteSequence key, UpdateFunction f);
  /**
   * Updates a record with the provided key according to the new value computed by the update function.
   *
   * @param treeName
   *          the tree name
   * @param key
   *          the key of the new record
   * @param f
   *          the update function
   * @return {@code true} if an update was performed, {@code false} otherwise
   * @see UpdateFunction#computeNewValue(ByteSequence)
   */
  boolean update(TreeName treeName, ByteSequence key, UpdateFunction f);

  boolean remove(TreeName name, ByteSequence key);

  void delete(TreeName name, ByteSequence key);
  /**
   * Deletes the record with the provided key, in the tree whose name is provided.
   *
   * @param treeName
   *          the tree name
   * @param key
   *          the key of the record to delete
   * @return {@code true} if the record could be deleted, {@code false} otherwise
   */
  boolean delete(TreeName treeName, ByteSequence key);
}