thiru-mg commented on code in PR #1639:
URL: https://github.com/apache/avro/pull/1639#discussion_r858275569


##########
lang/java/avro/src/main/java/org/apache/avro/file/DataFileStream.java:
##########
@@ -97,18 +97,30 @@ protected DataFileStream(DatumReader<D> reader) throws 
IOException {
     this.reader = reader;
   }
 
-  /** Initialize the stream by reading from its head. */
-  void initialize(InputStream in) throws IOException {
-    this.header = new Header();
-    this.vin = DecoderFactory.get().binaryDecoder(in, vin);
+  byte[] readMagic() throws IOException {
+    if (this.vin == null) {
+      throw new IOException("InputStream is not initialized");
+    }
     byte[] magic = new byte[DataFileConstants.MAGIC.length];
     try {
       vin.readFixed(magic); // read magic
     } catch (IOException e) {
       throw new IOException("Not an Avro data file.", e);
     }
+    return magic;
+  }
+
+  void validateMagic(byte[] magic) throws InvalidAvroMagicException {
     if (!Arrays.equals(DataFileConstants.MAGIC, magic))
       throw new InvalidAvroMagicException("Not an Avro data file.");
+  }
+
+  /** Initialize the stream by reading from its head. */
+  void initializeWithMagic(InputStream in, byte[] magic) throws IOException {

Review Comment:
   To my taste, I would have left this function named `initialize`. The reason 
is there is a pattern followed throughout to call the non-constructor 
initializers `initialize()`. But I wouldn't insist on it.



##########
lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java:
##########
@@ -128,15 +127,20 @@ public DataFileReader(File file, DatumReader<D> reader) 
throws IOException {
    * <pre/>
    */
   public DataFileReader(SeekableInput sin, DatumReader<D> reader) throws 
IOException {
-    this(sin, reader, false);
+    this(sin, reader, false, null);
+  }
+
+  public DataFileReader(SeekableInput sin, DatumReader<D> reader, byte[] 
magic) throws IOException {

Review Comment:
   Since the magic is embedded in the file and nobody outside the class (let 
alone the package) is likely to have the magic, it is better to make this 
constructor private. If we come across a valid use case in the future, we can 
then make it public.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to