[lucene] branch branch_9x updated: LUCENE-10369: Move DelegatingCacheHelper to FilterDirectoryReader (#596)

[jp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jpountz pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/lucene.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new 842ef16  LUCENE-10369: Move DelegatingCacheHelper to FilterDirectoryReader (#596)
842ef16 is described below

commit 842ef165e42bc7540676840697e13f28bf6e6230
Author: Nikola Grcevski <62...@users.noreply.github.com>
AuthorDate: Tue Jan 11 09:22:06 2022 -0500

    LUCENE-10369: Move DelegatingCacheHelper to FilterDirectoryReader (#596)
---
 .../apache/lucene/index/FilterDirectoryReader.java | 33 +++++++++++++
 .../index/SoftDeletesDirectoryReaderWrapper.java   | 22 ---------
 .../lucene/index/TestFilterDirectoryReader.java    | 54 ++++++++++++++++++++++
 3 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/lucene/core/src/java/org/apache/lucene/index/FilterDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/FilterDirectoryReader.java
index 42b8d24..725abda 100644
--- a/lucene/core/src/java/org/apache/lucene/index/FilterDirectoryReader.java
+++ b/lucene/core/src/java/org/apache/lucene/index/FilterDirectoryReader.java
@@ -147,4 +147,37 @@ public abstract class FilterDirectoryReader extends DirectoryReader {
   public DirectoryReader getDelegate() {
     return in;
   }
+
+  /**
+   * A DelegatingCacheHelper is a CacheHelper specialization for implementing long-lived caching
+   * behaviour for FilterDirectoryReader subclasses. It uses a unique CacheKey for the purpose of
+   * implementing the onClose listener delegation for the reader.
+   */
+  protected static class DelegatingCacheHelper implements CacheHelper {
+    private final CacheHelper delegate;
+    private final CacheKey cacheKey = new CacheKey();
+
+    /**
+     * Create a new DelegatingCacheHelper that delegates the cache onClose listener to another
+     * CacheHelper, but with its own unique CacheKey.
+     *
+     * @param delegate the CacheHelper to delegate the close listener to
+     */
+    protected DelegatingCacheHelper(CacheHelper delegate) {
+      this.delegate = delegate;
+    }
+
+    @Override
+    public CacheKey getKey() {
+      return cacheKey;
+    }
+
+    @Override
+    public void addClosedListener(ClosedListener listener) {
+      // here we wrap the listener and call it with our cache key
+      // this is important since this key will be used to cache the reader and otherwise we won't
+      // free caches etc.
+      delegate.addClosedListener(unused -> listener.onClose(cacheKey));
+    }
+  }
 }
diff --git a/lucene/core/src/java/org/apache/lucene/index/SoftDeletesDirectoryReaderWrapper.java b/lucene/core/src/java/org/apache/lucene/index/SoftDeletesDirectoryReaderWrapper.java
index fbf5171..e0998d21 100644
--- a/lucene/core/src/java/org/apache/lucene/index/SoftDeletesDirectoryReaderWrapper.java
+++ b/lucene/core/src/java/org/apache/lucene/index/SoftDeletesDirectoryReaderWrapper.java
@@ -259,26 +259,4 @@ public final class SoftDeletesDirectoryReaderWrapper extends FilterDirectoryRead
       return readerCacheHelper;
     }
   }
-
-  private static class DelegatingCacheHelper implements CacheHelper {
-    private final CacheHelper delegate;
-    private final CacheKey cacheKey = new CacheKey();
-
-    public DelegatingCacheHelper(CacheHelper delegate) {
-      this.delegate = delegate;
-    }
-
-    @Override
-    public CacheKey getKey() {
-      return cacheKey;
-    }
-
-    @Override
-    public void addClosedListener(ClosedListener listener) {
-      // here we wrap the listener and call it with our cache key
-      // this is important since this key will be used to cache the reader and otherwise we won't
-      // free caches etc.
-      delegate.addClosedListener(unused -> listener.onClose(cacheKey));
-    }
-  }
 }
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestFilterDirectoryReader.java b/lucene/core/src/test/org/apache/lucene/index/TestFilterDirectoryReader.java
index 620e5d3..6bd144d 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestFilterDirectoryReader.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestFilterDirectoryReader.java
@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.index.FilterDirectoryReader.SubReaderWrapper;
@@ -143,4 +144,57 @@ public class TestFilterDirectoryReader extends LuceneTestCase {
     directoryReader.close();
     dir.close();
   }
+
+  private static class DummyLastingFilterDirectoryReader extends FilterDirectoryReader {
+    private final CacheHelper cacheHelper;
+
+    public DummyLastingFilterDirectoryReader(DirectoryReader in) throws IOException {
+      super(in, new DummySubReaderWrapper());
+      cacheHelper =
+          (in.getReaderCacheHelper() == null)
+              ? null
+              : new DelegatingCacheHelper(in.getReaderCacheHelper());
+    }
+
+    @Override
+    protected DirectoryReader doWrapDirectoryReader(DirectoryReader in) throws IOException {
+      return new DummyFilterDirectoryReader(in);
+    }
+
+    @Override
+    public CacheHelper getReaderCacheHelper() {
+      return cacheHelper;
+    }
+  }
+
+  public void testDelegatingCacheHelper() throws IOException {
+    Directory dir = newDirectory();
+    IndexWriter w = new IndexWriter(dir, newIndexWriterConfig());
+    w.addDocument(new Document());
+
+    DirectoryReader reader = DirectoryReader.open(w);
+    DirectoryReader wrapped = new DummyLastingFilterDirectoryReader(reader);
+
+    assertNotEquals(reader.getReaderCacheHelper(), wrapped.getReaderCacheHelper());
+    assertNotEquals(
+        reader.getReaderCacheHelper().getKey(), wrapped.getReaderCacheHelper().getKey());
+
+    AtomicInteger closeCalledCounter = new AtomicInteger(0);
+
+    wrapped
+        .getReaderCacheHelper()
+        .addClosedListener(
+            key -> {
+              closeCalledCounter.incrementAndGet();
+              assertSame(key, wrapped.getReaderCacheHelper().getKey());
+            });
+
+    reader.close();
+    assertEquals(1, closeCalledCounter.get());
+    wrapped.close();
+    assertEquals(1, closeCalledCounter.get());
+
+    w.close();
+    dir.close();
+  }
 }