Fix NullPointerException in Monitor.getQuery when query is not present (

#12736)
apache · Oct 31, 2023 · 5694e30 · 5694e30
1 parent d44eef7
commit 5694e30
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 8 deletions.
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
@@ -138,6 +138,8 @@ Bug Fixes
 
 * GITHUB#12727: Ensure negative scores are not returned by vector similarity functions (Ben Trent)
 
+* GITHUB#12736: Fix NullPointerException when Monitor.getQuery cannot find the requested queryId (Davis Cook)
+
 Build
 ---------------------
 

diff --git a/lucene/monitor/src/java/org/apache/lucene/monitor/QueryIndex.java b/lucene/monitor/src/java/org/apache/lucene/monitor/QueryIndex.java
@@ -68,7 +68,7 @@ public MonitorQuery getQuery(String queryId) throws IOException {
     search(
         new TermQuery(new Term(FIELDS.query_id, queryId)),
         (id, query, dataValues) -> bytesHolder[0] = dataValues.mq.binaryValue());
-    return serializer.deserialize(bytesHolder[0]);
+    return bytesHolder[0] != null ? serializer.deserialize(bytesHolder[0]) : null;
   }
 
   public void scan(QueryCollector matcher) throws IOException {

diff --git a/lucene/monitor/src/test/org/apache/lucene/monitor/TestMonitorPersistence.java b/lucene/monitor/src/test/org/apache/lucene/monitor/TestMonitorPersistence.java
@@ -28,16 +28,21 @@ public class TestMonitorPersistence extends MonitorTestBase {
 
   private Path indexDirectory = createTempDir();
 
-  public void testCacheIsRepopulated() throws IOException {
-
-    Document doc = new Document();
-    doc.add(newTextField(FIELD, "test", Field.Store.NO));
+  protected Monitor newMonitorWithPersistence() throws IOException {
     MonitorConfiguration config =
         new MonitorConfiguration()
             .setIndexPath(
                 indexDirectory, MonitorQuerySerializer.fromParser(MonitorTestBase::parse));
 
-    try (Monitor monitor = new Monitor(ANALYZER, config)) {
+    return new Monitor(ANALYZER, config);
+  }
+
+  public void testCacheIsRepopulated() throws IOException {
+
+    Document doc = new Document();
+    doc.add(newTextField(FIELD, "test", Field.Store.NO));
+
+    try (Monitor monitor = newMonitorWithPersistence()) {
       monitor.register(
           mq("1", "test"),
           mq("2", "test"),
@@ -58,7 +63,7 @@ public void testCacheIsRepopulated() throws IOException {
           e.getMessage());
     }
 
-    try (Monitor monitor2 = new Monitor(ANALYZER, config)) {
+    try (Monitor monitor2 = newMonitorWithPersistence()) {
       assertEquals(4, monitor2.getQueryCount());
       assertEquals(4, monitor2.match(doc, QueryMatch.SIMPLE_MATCHER).getMatchCount());
 
@@ -67,9 +72,24 @@ public void testCacheIsRepopulated() throws IOException {
     }
   }
 
+  public void testGetQueryPresent() throws IOException {
+    try (Monitor monitor = newMonitorWithPersistence()) {
+      MonitorQuery monitorQuery = mq("1", "test");
+      monitor.register(monitorQuery);
+
+      assertEquals(monitorQuery, monitor.getQuery("1"));
+    }
+  }
+
+  public void testGetQueryNotPresent() throws IOException {
+    try (Monitor monitor = newMonitorWithPersistence()) {
+      assertNull(monitor.getQuery("1"));
+    }
+  }
+
   public void testEphemeralMonitorDoesNotStoreQueries() throws IOException {
 
-    try (Monitor monitor2 = new Monitor(ANALYZER)) {
+    try (Monitor monitor2 = newMonitor(ANALYZER)) {
       IllegalStateException e =
           expectThrows(IllegalStateException.class, () -> monitor2.getQuery("query"));
       assertEquals(