more javadoc and tests

Author: normen662

fdb-extensions/src/main/java/com/apple/foundationdb/async/hnsw/AbstractNode.java

@@ -27,8 +27,14 @@
 import java.util.List;
 
 /**
- * TODO.
- * @param <N> node type class.
+ * An abstract base class implementing the {@link Node} interface.
+ * <p>
+ * This class provides the fundamental structure for a node within the HNSW graph,
+ * managing a unique {@link Tuple} primary key and an immutable list of its neighbors.
+ * Subclasses are expected to provide concrete implementations, potentially adding
+ * more state or behavior.
+ *
+ * @param <N> the type of the node reference used for neighbors, which must extend {@link NodeReference}
  */
 abstract class AbstractNode<N extends NodeReference> implements Node<N> {
     @Nonnull
@@ -37,24 +43,53 @@ abstract class AbstractNode<N extends NodeReference> implements Node<N> {
     @Nonnull
     private final List<N> neighbors;
 
+    /**
+     * Constructs a new {@code AbstractNode} with a specified primary key and a list of neighbors.
+     * <p>
+     * This constructor creates a defensive, immutable copy of the provided {@code neighbors} list.
+     * This ensures that the internal state of the node cannot be modified by external
+     * changes to the original list after construction.
+     *
+     * @param primaryKey the unique identifier for this node; must not be {@code null}
+     * @param neighbors the list of nodes connected to this node; must not be {@code null}
+     */
     protected AbstractNode(@Nonnull final Tuple primaryKey,
                            @Nonnull final List<N> neighbors) {
         this.primaryKey = primaryKey;
         this.neighbors = ImmutableList.copyOf(neighbors);
     }
 
+    /**
+     * Gets the primary key that uniquely identifies this object.
+     * @return the primary key {@link  Tuple}, which will never be {@code null}.
+     */
     @Nonnull
     @Override
     public Tuple getPrimaryKey() {
         return primaryKey;
     }
 
+    /**
+     * Gets the list of neighbors connected to this node.
+     * <p>
+     * This method returns a direct reference to the internal list which is
+     * immutable.
+     * @return a non-null, possibly empty, list of neighbors.
+     */
     @Nonnull
     @Override
     public List<N> getNeighbors() {
         return neighbors;
     }
 
+    /**
+     * Gets the neighbor at the specified index.
+     * <p>
+     * This method provides access to a specific neighbor by its zero-based position
+     * in the internal list of neighbors.
+     * @param index the zero-based index of the neighbor to retrieve.
+     * @return the neighbor at the specified index, guaranteed to be non-null.
+     */
     @Nonnull
     @Override
     public N getNeighbor(final int index) {

fdb-extensions/src/main/java/com/apple/foundationdb/async/hnsw/AbstractStorageAdapter.java

@@ -32,7 +32,14 @@
 import java.util.concurrent.CompletableFuture;
 
 /**
- * Implementations and attributes common to all concrete implementations of {@link StorageAdapter}.
+ * An abstract base class for {@link StorageAdapter} implementations.
+ * <p>
+ * This class provides the common infrastructure for managing HNSW graph data within {@link Subspace}.
+ * It handles the configuration, node creation, and listener management, while delegating the actual
+ * storage-specific read and write operations to concrete subclasses through the {@code fetchNodeInternal}
+ * and {@code writeNodeInternal} abstract methods.
+ *
+ * @param <N> the type of {@link NodeReference} used to reference nodes in the graph
  */
 abstract class AbstractStorageAdapter<N extends NodeReference> implements StorageAdapter<N> {
     @Nonnull
@@ -51,6 +58,19 @@ abstract class AbstractStorageAdapter<N extends NodeReference> implements Storag
 
     private final Subspace dataSubspace;
 
+    /**
+     * Constructs a new {@code AbstractStorageAdapter}.
+     * <p>
+     * This constructor initializes the adapter with the necessary configuration,
+     * factories, and listeners for managing an HNSW graph. It also sets up a
+     * dedicated data subspace within the provided main subspace for storing node data.
+     *
+     * @param config the HNSW graph configuration
+     * @param nodeFactory the factory to create new nodes of type {@code <N>}
+     * @param subspace the primary subspace for storing all graph-related data
+     * @param onWriteListener the listener to be called on write operations
+     * @param onReadListener the listener to be called on read operations
+     */
     protected AbstractStorageAdapter(@Nonnull final HNSW.Config config, @Nonnull final NodeFactory<N> nodeFactory,
                                      @Nonnull final Subspace subspace,
                                      @Nonnull final OnWriteListener onWriteListener,
@@ -63,55 +83,138 @@ protected AbstractStorageAdapter(@Nonnull final HNSW.Config config, @Nonnull fin
         this.dataSubspace = subspace.subspace(Tuple.from(SUBSPACE_PREFIX_DATA));
     }
 
+    /**
+     * Returns the configuration used to build and search this HNSW graph.
+     *
+     * @return the current {@link HNSW.Config} object, never {@code null}.
+     */
     @Override
     @Nonnull
     public HNSW.Config getConfig() {
         return config;
     }
 
+    /**
+     * Gets the factory responsible for creating new nodes.
+     * <p>
+     * This factory is used to instantiate nodes of the generic type {@code N}
+     * for the current context. The {@code @Nonnull} annotation guarantees that
+     * this method will never return {@code null}.
+     *
+     * @return the non-null {@link NodeFactory} instance.
+     */
     @Nonnull
     @Override
     public NodeFactory<N> getNodeFactory() {
         return nodeFactory;
     }
 
+    /**
+     * Gets the kind of this node, which uniquely identifies the type of node.
+     * <p>
+     * This method is an override and provides a way to determine the concrete
+     * type of node without using {@code instanceof} checks.
+     *
+     * @return the non-null {@link NodeKind} representing the type of this node.
+     */
     @Nonnull
     @Override
     public NodeKind getNodeKind() {
         return getNodeFactory().getNodeKind();
     }
 
+    /**
+     * Gets the subspace in which this key or value is stored.
+     * <p>
+     * This subspace provides a logical separation for keys within the underlying key-value store.
+     *
+     * @return the non-null {@link Subspace} for this context
+     */
     @Override
     @Nonnull
     public Subspace getSubspace() {
         return subspace;
     }
 
+    /**
+     * Gets the subspace for the data associated with this component.
+     * <p>
+     * The data subspace defines the portion of the directory space where the data
+     * for this component is stored.
+     *
+     * @return the non-null {@link Subspace} for the data
+     */
     @Override
     @Nonnull
     public Subspace getDataSubspace() {
         return dataSubspace;
     }
 
+    /**
+     * Returns the listener that is notified upon write events.
+     * <p>
+     * This method is an override and guarantees a non-null return value,
+     * as indicated by the {@code @Nonnull} annotation.
+     *
+     * @return the configured {@link OnWriteListener} instance; will never be {@code null}.
+     */
     @Override
     @Nonnull
     public OnWriteListener getOnWriteListener() {
         return onWriteListener;
     }
 
+    /**
+     * Gets the listener that is notified upon completion of a read operation.
+     * <p>
+     * This method is an override and provides the currently configured listener instance.
+     * The returned listener is guaranteed to be non-null as indicated by the
+     * {@code @Nonnull} annotation.
+     *
+     * @return the non-null {@link OnReadListener} instance.
+     */
     @Override
     @Nonnull
     public OnReadListener getOnReadListener() {
         return onReadListener;
     }
 
+    /**
+     * Asynchronously fetches a node from a specific layer of the HNSW.
+     * <p>
+     * The node is identified by its {@code layer} and {@code primaryKey}. The entire fetch operation is
+     * performed within the given {@link ReadTransaction}. After the underlying
+     * fetch operation completes, the retrieved node is validated by the
+     * {@link  #checkNode(Node)} method before the returned future is completed.
+     *
+     * @param readTransaction the non-null transaction to use for the read operation
+     * @param layer the layer of the tree from which to fetch the node
+     * @param primaryKey the non-null primary key that identifies the node to fetch
+     *
+     * @return a {@link CompletableFuture} that will complete with the fetched {@link Node}
+     * once it has been read from storage and validated
+     */
     @Nonnull
     @Override
     public CompletableFuture<Node<N>> fetchNode(@Nonnull final ReadTransaction readTransaction,
                                                 int layer, @Nonnull Tuple primaryKey) {
         return fetchNodeInternal(readTransaction, layer, primaryKey).thenApply(this::checkNode);
     }
 
+    /**
+     * Asynchronously fetches a specific node from the data store for a given layer and primary key.
+     * <p>
+     * This is an internal, abstract method that concrete subclasses must implement to define
+     * the storage-specific logic for retrieving a node. The operation is performed within the
+     * context of the provided {@link ReadTransaction}.
+     *
+     * @param readTransaction the transaction to use for the read operation; must not be {@code null}
+     * @param layer the layer index from which to fetch the node
+     * @param primaryKey the primary key that uniquely identifies the node to be fetched; must not be {@code null}
+     *
+     * @return a {@link CompletableFuture} that will be completed with the fetched {@link Node}.
+     * The future will complete with {@code null} if no node is found for the given key and layer.
+     */
     @Nonnull
     protected abstract CompletableFuture<Node<N>> fetchNodeInternal(@Nonnull ReadTransaction readTransaction,
                                                                     int layer, @Nonnull Tuple primaryKey);
@@ -129,6 +232,21 @@ private Node<N> checkNode(@Nullable final Node<N> node) {
         return node;
     }
 
+    /**
+     * Writes a given node and its neighbor modifications to the underlying storage.
+     * <p>
+     * This operation is executed within the context of the provided {@link Transaction}.
+     * It handles persisting the node's data at a specific {@code layer} and applies
+     * the changes to its neighbors as defined in the {@link NeighborsChangeSet}.
+     * This method delegates the core writing logic to an internal method and provides
+     * debug logging upon completion.
+     *
+     * @param transaction the non-null {@link Transaction} context for this write operation
+     * @param node the non-null {@link Node} to be written to storage
+     * @param layer the layer index where the node is being written
+     * @param changeSet the non-null {@link NeighborsChangeSet} detailing the modifications
+     * to the node's neighbors
+     */
     @Override
     public void writeNode(@Nonnull Transaction transaction, @Nonnull Node<N> node, int layer,
                           @Nonnull NeighborsChangeSet<N> changeSet) {
@@ -138,6 +256,20 @@ public void writeNode(@Nonnull Transaction transaction, @Nonnull Node<N> node, i
         }
     }
 
+    /**
+     * Writes a single node to the data store as part of a larger transaction.
+     * <p>
+     * This is an abstract method that concrete implementations must provide.
+     * It is responsible for the low-level persistence of the given {@code node} at a
+     * specific {@code layer}. The implementation should also handle the modifications
+     * to the node's neighbors, as detailed in the {@code changeSet}.
+     *
+     * @param transaction the non-null transaction context for the write operation
+     * @param node the non-null {@link Node} to write
+     * @param layer the layer or level of the node in the structure
+     * @param changeSet the non-null {@link NeighborsChangeSet} detailing additions or
+     * removals of neighbor links
+     */
     protected abstract void writeNodeInternal(@Nonnull Transaction transaction, @Nonnull Node<N> node, int layer,
                                               @Nonnull NeighborsChangeSet<N> changeSet);
 

fdb-extensions/src/main/java/com/apple/foundationdb/async/hnsw/BaseNeighborsChangeSet.java

@@ -30,28 +30,62 @@
 import java.util.function.Predicate;
 
 /**
- * TODO.
+ * A base implementation of the {@link NeighborsChangeSet} interface.
+ * <p>
+ * This class represents a complete, non-delta state of a node's neighbors. It holds a fixed, immutable
+ * list of neighbors provided at construction time. As such, it does not support parent change sets or writing deltas.
+ *
+ * @param <N> the type of the node reference, which must extend {@link NodeReference}
  */
 class BaseNeighborsChangeSet<N extends NodeReference> implements NeighborsChangeSet<N> {
     @Nonnull
     private final List<N> neighbors;
 
+    /**
+     * Creates a new change set with the specified neighbors.
+     * <p>
+     * This constructor creates an immutable copy of the provided list.
+     *
+     * @param neighbors the list of neighbors for this change set; must not be null.
+     */
     public BaseNeighborsChangeSet(@Nonnull final List<N> neighbors) {
         this.neighbors = ImmutableList.copyOf(neighbors);
     }
 
+    /**
+     * Gets the parent change set.
+     * <p>
+     * This implementation always returns {@code null}, as this type of change set
+     * does not have a parent.
+     *
+     * @return always {@code null}.
+     */
     @Nullable
     @Override
     public BaseNeighborsChangeSet<N> getParent() {
         return null;
     }
 
+    /**
+     * Retrieves the list of neighbors associated with this object.
+     * <p>
+     * This implementation fulfills the {@code merge} contract by simply returning the
+     * existing list of neighbors without performing any additional merging logic.
+     * @return a non-null list of neighbors. The generic type {@code N} represents
+     *         the type of the neighboring elements.
+     */
     @Nonnull
     @Override
     public List<N> merge() {
         return neighbors;
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * <p>This implementation is a no-op and does not write any delta information,
+     * as indicated by the empty method body.
+     */
     @Override
     public void writeDelta(@Nonnull final InliningStorageAdapter storageAdapter, @Nonnull final Transaction transaction,
                            final int layer, @Nonnull final Node<N> node,