ChrisHegarty commented on code in PR #14482:
URL: https://github.com/apache/lucene/pull/14482#discussion_r2044187179


##########
lucene/core/src/test/org/apache/lucene/store/TestDirectory.java:
##########
@@ -126,4 +126,27 @@ public void testListAll() throws Throwable {
     assertTrue(files.contains(file1.getFileName().toString()));
     assertTrue(files.contains(file2.getFileName().toString()));
   }
+
+  public void testValidateIOContext() throws Exception {

Review Comment:
   we can add this to `BaseDirectoryTestCase`, and use ` try (Directory dir = 
newDirectory()) { ...` to test all implementations.



##########
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##########
@@ -44,71 +37,125 @@ public enum Context {
     DEFAULT
   };
 
+  /** Implemented by classes that can specify hints on how the file will be 
used */
+  interface FileOpenHint {}
+
   /**
    * A default context for normal reads/writes. Use {@link 
#withReadAdvice(ReadAdvice)} to specify
    * another {@link ReadAdvice}.
    *
    * <p>It will use {@link ReadAdvice#RANDOM} by default, unless set by system 
property {@code
    * org.apache.lucene.store.defaultReadAdvice}.
    */
-  public static final IOContext DEFAULT = new 
IOContext(Constants.DEFAULT_READADVICE);
+  IOContext DEFAULT = new DefaultIOContext(Constants.DEFAULT_READADVICE);
 
   /**
    * A default context for reads with {@link ReadAdvice#SEQUENTIAL}.
    *
    * <p>This context should only be used when the read operations will be 
performed in the same
    * thread as the thread that opens the underlying storage.
    */
-  public static final IOContext READONCE = new 
IOContext(ReadAdvice.SEQUENTIAL);
-
-  @SuppressWarnings("incomplete-switch")
-  public IOContext {
-    Objects.requireNonNull(context, "context must not be null");
-    Objects.requireNonNull(readAdvice, "readAdvice must not be null");
-    switch (context) {
-      case MERGE ->
-          Objects.requireNonNull(mergeInfo, "mergeInfo must not be null if 
context is MERGE");
-      case FLUSH ->
-          Objects.requireNonNull(flushInfo, "flushInfo must not be null if 
context is FLUSH");
-    }
-    if ((context == Context.FLUSH || context == Context.MERGE)
-        && readAdvice != ReadAdvice.SEQUENTIAL) {
-      throw new IllegalArgumentException(
-          "The FLUSH and MERGE contexts must use the SEQUENTIAL read access 
advice");
-    }
-  }
+  IOContext READONCE = new DefaultIOContext(ReadAdvice.SEQUENTIAL);
 
-  /** Creates a default {@link IOContext} for reading/writing with the given 
{@link ReadAdvice} */
-  private IOContext(ReadAdvice accessAdvice) {
-    this(Context.DEFAULT, null, null, accessAdvice);
+  static IOContext defaultContext(FileOpenHint... hints) {
+    return new DefaultIOContext(Constants.DEFAULT_READADVICE, hints);
   }
 
-  /** Creates an {@link IOContext} for flushing. */
-  public IOContext(FlushInfo flushInfo) {
-    this(Context.FLUSH, null, flushInfo, ReadAdvice.SEQUENTIAL);
+  /** Returns an {@link IOContext} for merging with the specified {@link 
MergeInfo} */
+  static IOContext merge(MergeInfo mergeInfo) {
+    Objects.requireNonNull(mergeInfo);
+    return new IOContext() {
+      @Override
+      public Context context() {
+        return Context.MERGE;
+      }
+
+      @Override
+      public MergeInfo mergeInfo() {
+        return mergeInfo;
+      }
+
+      @Override
+      public FlushInfo flushInfo() {
+        return null;
+      }
+
+      @Override
+      public FileOpenHint[] hints() {
+        return new FileOpenHint[0];
+      }
+
+      @Override
+      public ReadAdvice readAdvice() {
+        return ReadAdvice.SEQUENTIAL;
+      }
+
+      @Override
+      public IOContext withReadAdvice(ReadAdvice advice) {
+        return this;
+      }
+    };
   }
 
-  /** Creates an {@link IOContext} for merging. */
-  public IOContext(MergeInfo mergeInfo) {
-    // Merges read input segments sequentially.
-    this(Context.MERGE, mergeInfo, null, ReadAdvice.SEQUENTIAL);
+  /** Returns an {@link IOContext} for flushing with the specified {@link 
FlushInfo} */
+  static IOContext flush(FlushInfo flushInfo) {
+    Objects.requireNonNull(flushInfo);
+    return new IOContext() {
+      @Override
+      public Context context() {
+        return Context.FLUSH;
+      }
+
+      @Override
+      public MergeInfo mergeInfo() {
+        return null;
+      }
+
+      @Override
+      public FlushInfo flushInfo() {
+        return flushInfo;
+      }
+
+      @Override
+      public FileOpenHint[] hints() {
+        return new FileOpenHint[0];
+      }
+
+      @Override
+      public ReadAdvice readAdvice() {
+        return ReadAdvice.SEQUENTIAL;
+      }
+
+      @Override
+      public IOContext withReadAdvice(ReadAdvice advice) {
+        return this;
+      }
+    };
   }
 
-  private static final IOContext[] READADVICE_TO_IOCONTEXT =
-      
Arrays.stream(ReadAdvice.values()).map(IOContext::new).toArray(IOContext[]::new);
+  /** The {@link Context} this context is for */
+  Context context();
+
+  /** Merge info, if {@link #context()} is {@link Context#MERGE} */
+  MergeInfo mergeInfo();
+
+  /** Flush info, if {@link #context()} is {@link Context#FLUSH} */
+  FlushInfo flushInfo();
+
+  /** Any hints on how the file will be opened */
+  FileOpenHint[] hints();

Review Comment:
   My pref here would be for an immutable structure, so an unmodifiable Set or 
List.



-- 
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: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to