1. Rename MetadataParser to MetadataContentParser and MetadataManager to MetadataParser

2. Add TODOs and Deprecate non-generic methods

Signed-off-by: Varun Bansal <bansvaru@amazon.com>

Author: linuxpi

server/src/main/java/org/opensearch/common/metadata/MetadataContentParser.java

@@ -0,0 +1,45 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.common.metadata;
+
+import org.apache.lucene.store.IndexInput;
+import org.apache.lucene.store.IndexOutput;
+
+import java.io.IOException;
+import java.util.Map;
+
+/**
+ * Parser interface for Metadata. Holds methods to convert to/from file content streams to metadata object holder - {@link T}
+ * @param <T> The type of metadata to be parsed
+ */
+public interface MetadataContentParser<T> {
+    /**
+     * Implements logic to read metadata content from metadata file input stream {@code indexInput} and parse into {@link T}
+     * @param indexInput metadata file input stream
+     * @return metadata content parsed to {@link T}
+     */
+    T readContent(IndexInput indexInput) throws IOException;
+
+    /**
+     * Implements logic to write metadata content from {@code content} to metadata file output stream {@code indexOutput}
+     * @param indexOutput metadata file input stream
+     */
+    void writeContent(IndexOutput indexOutput, T content) throws IOException;
+
+    /**
+     * Implements logic to write metadata content from {@code content} to metadata file output stream {@code indexOutput}
+     * This method only supports metadata content in the form {@code Map<String, String>} to support RemoteSegment store metadata content.
+     * @param indexOutput metadata file input stream
+     * @param content metadata content
+     *
+     * TODO - This will removed in future releases and only {@link #writeContent(IndexOutput, Object)} should be used.
+     */
+    @Deprecated
+    void writeContent(IndexOutput indexOutput, Map<String, String> content) throws IOException;
+}

server/src/main/java/org/opensearch/common/metadata/MetadataManager.java

@@ -8,37 +8,107 @@
 
 package org.opensearch.common.metadata;
 
-import org.apache.lucene.store.IndexInput;
-import org.apache.lucene.store.IndexOutput;
-
 import java.io.IOException;
 import java.util.Map;
 
+import org.apache.lucene.codecs.CodecUtil;
+import org.apache.lucene.store.BufferedChecksumIndexInput;
+import org.apache.lucene.store.ChecksumIndexInput;
+import org.apache.lucene.store.IndexInput;
+import org.apache.lucene.store.IndexOutput;
+
 /**
- * Parser interface for Metadata. Holds methods to convert to/from file content streams to metadata object holder - {@link T}
- * @param <T> The type of metadata to be parsed
+ * Manages Metadata by adding version, codec in header and checksum in footer.
+ * @param <T> The type of metadata to be read/written
  */
-public interface MetadataParser<T> {
+public class MetadataParser<T> {
+    // This can be updated to hold a parserFactory and get relevant parsers based on the metadata versions
+    private final MetadataContentParser<T> parser;
+    private final int currentVersion;
+    private final String codec;
+
     /**
-     * Implements logic to read metadata content from metadata file input stream {@code indexInput} and parse into {@link T}
-     * @param indexInput metadata file input stream
-     * @return metadata content parsed to {@link T}
+     * @param parser parser to read/write metadata from T
+     * @param currentVersion latest supported version of the metadata
+     * @param codec: metadata codec
      */
-    T readContent(IndexInput indexInput) throws IOException;
+    public MetadataParser(MetadataContentParser<T> parser, int currentVersion, String codec) {
+        this.parser = parser;
+        this.currentVersion = currentVersion;
+        this.codec = codec;
+    }
 
     /**
-     * Implements logic to write metadata content from {@code content} to metadata file output stream {@code indexOutput}
-     * @param indexOutput metadata file input stream
+     * Reads metadata content from {@code indexInput} and parses the read content to {@link T}.
+     * Before reading actual content, verifies the header with relevant codec and version.
+     * After reading the actual content, verifies the checksum as well
+     * @param indexInput metadata file input stream
+     * @return metadata content parsed into {@link T}
      */
-    void writeContent(IndexOutput indexOutput, T content) throws IOException;
+    public T readMetadata(IndexInput indexInput) throws IOException {
+        ChecksumIndexInput checksumIndexInput = new BufferedChecksumIndexInput(indexInput);
+        checkHeader(checksumIndexInput);
+        T metadata = this.parser.readContent(checksumIndexInput);
+        checkFooter(checksumIndexInput);
+        return metadata;
+    }
 
     /**
-     * Implements logic to write metadata content from {@code content} to metadata file output stream {@code indexOutput}
-     * This method only supports metadata content in the form {@code Map<String, String>} to support RemoteSegment store metadata content.
-     * This will removed in future releases and only {@link #writeContent(IndexOutput, Object)} )} should be used going fwd.
-     * @param indexOutput metadata file input stream
-     * @param content metadata content
+     * Writes metadata to file output stream {@code indexOutput}
+     * @param indexOutput file output stream which will store metadata content
+     * @param metadata metadata content.
+     *
+     * TODO - This will removed in future releases and only {@link #writeMetadata(IndexOutput, Object)} should be used.
      */
     @Deprecated
-    void writeContent(IndexOutput indexOutput, Map<String, String> content) throws IOException;
+    public void writeMetadata(IndexOutput indexOutput, Map<String, String> metadata) throws IOException {
+        this.writeHeader(indexOutput);
+        this.parser.writeContent(indexOutput, metadata);
+        this.writeFooter(indexOutput);
+    }
+
+    /**
+     * Writes metadata to file output stream {@code indexOutput}
+     * @param indexOutput file output stream which will store metadata content
+     * @param metadata metadata content.
+     */
+    public void writeMetadata(IndexOutput indexOutput, T metadata) throws IOException {
+        this.writeHeader(indexOutput);
+        this.parser.writeContent(indexOutput, metadata);
+        this.writeFooter(indexOutput);
+    }
+
+    /**
+     * Reads header from metadata file input stream containing {@code this.codec} and {@code this.currentVersion}.
+     * @param indexInput metadata file input stream
+     * @return header version found in the metadata file
+     */
+    private int checkHeader(IndexInput indexInput) throws IOException {
+        return CodecUtil.checkHeader(indexInput, this.codec, this.currentVersion, this.currentVersion);
+    }
+
+    /**
+     * Reads footer from metadata file input stream containing checksum.
+     * The {@link IndexInput#getFilePointer()} should be at the footer start position.
+     * @param indexInput metadata file input stream
+     */
+    private void checkFooter(ChecksumIndexInput indexInput) throws IOException {
+        CodecUtil.checkFooter(indexInput);
+    }
+
+    /**
+     * Writes header with {@code this.codec} and {@code this.currentVersion} to the metadata file output stream
+     * @param indexOutput metadata file output stream
+     */
+    private void writeHeader(IndexOutput indexOutput) throws IOException {
+        CodecUtil.writeHeader(indexOutput, this.codec, this.currentVersion);
+    }
+
+    /**
+     * Writes footer with checksum of contents of metadata file output stream
+     * @param indexOutput metadata file output stream
+     */
+    private void writeFooter(IndexOutput indexOutput) throws IOException {
+        CodecUtil.writeFooter(indexOutput);
+    }
 }

server/src/main/java/org/opensearch/common/metadata/MetadataParser.java

@@ -18,7 +18,7 @@
 import org.apache.lucene.store.IndexOutput;
 import org.opensearch.common.UUIDs;
 import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadata;
-import org.opensearch.common.metadata.MetadataManager;
+import org.opensearch.common.metadata.MetadataParser;
 
 import java.io.IOException;
 import java.nio.file.NoSuchFileException;
@@ -77,19 +77,19 @@ public final class RemoteSegmentStoreDirectory extends FilterDirectory {
      */
     private Map<String, UploadedSegmentMetadata> segmentsUploadedToRemoteStore;
 
-    private MetadataManager<RemoteSegmentMetadata> remoteMetadataManager;
+    private MetadataParser<RemoteSegmentMetadata> remoteMetadataParser;
 
     private static final Logger logger = LogManager.getLogger(RemoteSegmentStoreDirectory.class);
 
     public RemoteSegmentStoreDirectory(
         RemoteDirectory remoteDataDirectory,
         RemoteDirectory remoteMetadataDirectory,
-        MetadataManager<RemoteSegmentMetadata> metadataManager
+        MetadataParser<RemoteSegmentMetadata> metadataParser
     ) throws IOException {
         super(remoteDataDirectory);
         this.remoteDataDirectory = remoteDataDirectory;
         this.remoteMetadataDirectory = remoteMetadataDirectory;
-        this.remoteMetadataManager = metadataManager;
+        this.remoteMetadataParser = metadataParser;
         init();
     }
 
@@ -135,7 +135,7 @@ private Map<String, UploadedSegmentMetadata> readLatestMetadataFile() throws IOE
 
     private Map<String, UploadedSegmentMetadata> readMetadataFile(String metadataFilename) throws IOException {
         try (IndexInput indexInput = remoteMetadataDirectory.openInput(metadataFilename, IOContext.DEFAULT)) {
-            RemoteSegmentMetadata metadata = this.remoteMetadataManager.readMetadata(indexInput);
+            RemoteSegmentMetadata metadata = this.remoteMetadataParser.readMetadata(indexInput);
             return metadata.getMetadata();
         }
     }
@@ -361,7 +361,7 @@ public void uploadMetadata(Collection<String> segmentFiles, Directory storeDirec
                     throw new NoSuchFileException(file);
                 }
             }
-            this.remoteMetadataManager.writeMetadata(indexOutput, uploadedSegments);
+            this.remoteMetadataParser.writeMetadata(indexOutput, uploadedSegments);
             indexOutput.close();
             storeDirectory.sync(Collections.singleton(metadataFilename));
             remoteMetadataDirectory.copyFrom(storeDirectory, metadataFilename, metadataFilename, IOContext.DEFAULT);

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

@@ -48,7 +48,7 @@ public Directory newDirectory(String repositoryName, IndexSettings indexSettings
             RemoteDirectory dataDirectory = createRemoteDirectory(repository, commonBlobPath, "data");
             RemoteDirectory metadataDirectory = createRemoteDirectory(repository, commonBlobPath, "metadata");
 
-            return new RemoteSegmentStoreDirectory(dataDirectory, metadataDirectory, RemoteSegmentMetadata.createMetadataManager());
+            return new RemoteSegmentStoreDirectory(dataDirectory, metadataDirectory, RemoteSegmentMetadata.createMetadataParser());
         } catch (RepositoryMissingException e) {
             throw new IllegalArgumentException("Repository should be created before creating index with remote_store enabled setting", e);
         }

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryFactory.java

@@ -11,7 +11,7 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import org.opensearch.common.metadata.MetadataManager;
+import org.opensearch.common.metadata.MetadataParser;
 import org.opensearch.index.store.RemoteSegmentStoreDirectory;
 
 /**
@@ -63,10 +63,10 @@ public static RemoteSegmentMetadata fromMapOfStrings(Map<String, String> segment
     }
 
     /**
-     * Constructs {@link MetadataManager} for {@link RemoteSegmentMetadata}
-     * @return {@link MetadataManager}
+     * Constructs {@link MetadataParser} for {@link RemoteSegmentMetadata}
+     * @return {@link MetadataParser}
      */
-    public static MetadataManager<RemoteSegmentMetadata> createMetadataManager() {
-        return new MetadataManager<>(new RemoteSegmentMetadataParser(), CURRENT_VERSION, METADATA_CODEC);
+    public static MetadataParser<RemoteSegmentMetadata> createMetadataParser() {
+        return new MetadataParser<>(new RemoteSegmentMetadataContentParser(), CURRENT_VERSION, METADATA_CODEC);
     }
 }

server/src/main/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadata.java

@@ -13,12 +13,12 @@
 
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.IndexOutput;
-import org.opensearch.common.metadata.MetadataParser;
+import org.opensearch.common.metadata.MetadataContentParser;
 
 /**
  * Parser for {@link RemoteSegmentMetadata}
  */
-public class RemoteSegmentMetadataParser implements MetadataParser<RemoteSegmentMetadata> {
+public class RemoteSegmentMetadataContentParser implements MetadataContentParser<RemoteSegmentMetadata> {
     /**
      * Reads metadata content from metadata file input stream and parsed into {@link RemoteSegmentMetadata}
      * @param indexInput metadata file input stream with {@link IndexInput#getFilePointer()} pointing to metadata content
@@ -41,9 +41,6 @@ public void writeContent(IndexOutput indexOutput, RemoteSegmentMetadata content)
      * Writes metadata to file output stream
      * @param indexOutput metadata file input stream
      * @param content metadata content
-     *
-     * This method would be removed in future
-     * and only {@link #writeContent(IndexOutput, RemoteSegmentMetadata)} would be leveraged in future
      */
     @Override
     public void writeContent(IndexOutput indexOutput, Map<String, String> content) throws IOException {

…etadata/RemoteSegmentMetadataParser.java …/RemoteSegmentMetadataContentParser.javaserver/src/main/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataParser.java renamed to server/src/main/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataContentParser.java server/src/main/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataParser.java renamed to server/src/main/java/org/opensearch/index/store/remote/metadata/RemoteSegmentMetadataContentParser.java

@@ -60,7 +60,7 @@ public void setup() throws IOException {
         remoteSegmentStoreDirectory = new RemoteSegmentStoreDirectory(
             remoteDataDirectory,
             remoteMetadataDirectory,
-            RemoteSegmentMetadata.createMetadataManager()
+            RemoteSegmentMetadata.createMetadataParser()
         );
     }