Fix #1608: rebuild match graph from DB at startup, stop resurrecting deleted spot maps

src/main/java/org/ecocean/StartupWildbook.java

@@ -886,9 +886,6 @@ public void contextDestroyed(ServletContextEvent sce) {
         System.out.println("* StartupWildbook destroyed called for: " +
             servletContextInfo(sContext));
 
-        if (CommonConfiguration.useSpotPatternRecognition(context)) {
-            saveMatchGraph(sContext, context);
-        }
         // Stop the WBIA poller first so it does not race teardown of
         // Shepherd / IndexingManager / QueueUtil while a poll cycle is in
         // flight. shutdownWbiaRegisterExecutor signals shutdown by
@@ -938,30 +935,18 @@ public static void createMatchGraph() {
      * via MatchGraphCreationThread.
      */
     public static void loadMatchGraphOrRebuild(ServletContext sContext, String context) {
-        try {
-            String dataDir = CommonConfiguration.getDataDirectory(sContext, context).getAbsolutePath();
-            String cacheFile = GridManager.getCacheFilePath(dataDir);
-            if (new File(cacheFile).exists() && GridManager.cacheRead(cacheFile)) {
-                System.out.println("INFO: matchGraph loaded from cache.");
-                return;
-            }
-        } catch (Exception e) {
-            System.out.println("WARNING: Could not load matchGraph cache, rebuilding from DB: " +
-                e.getMessage());
-        }
+        // Always rebuild the match graph from the database at startup. The match
+        // graph is the candidate set for Modified-Groth scans; rebuilding from the
+        // DB on every startup guarantees deleted spot maps / encounters cannot
+        // reappear as match candidates (issue #1608). The former
+        // WEB-INF/MatchGraphCache.json startup cache was only refreshed on a
+        // graceful shutdown, so a crash/kill left it stale and the next startup
+        // resurrected deleted spots. setMatchGraphReady(false) up front keeps
+        // scans gated until the rebuild completes.
+        GridManager.setMatchGraphReady(false);
         createMatchGraph();
     }
 
-    public static void saveMatchGraph(ServletContext sContext, String context) {
-        try {
-            String dataDir = CommonConfiguration.getDataDirectory(sContext, context).getAbsolutePath();
-            String cacheFile = GridManager.getCacheFilePath(dataDir);
-            GridManager.cacheWrite(cacheFile);
-        } catch (Exception e) {
-            System.out.println("WARNING: Could not save matchGraph cache: " + e.getMessage());
-        }
-    }
-
     public static boolean skipInit(ServletContextEvent sce, String extra) {
         ServletContext sc = sce.getServletContext();
 

src/main/java/org/ecocean/grid/GridManager.java

@@ -2,21 +2,17 @@
 
 // import org.apache.commons.math.stat.descriptive.SummaryStatistics;
 // import org.ecocean.CommonConfiguration;
-import org.ecocean.Util;
 import org.ecocean.shepherd.core.Shepherd;
 
 // import org.ecocean.servlet.ServletUtilities;
 
-import java.io.IOException;
-import java.util.Iterator;
 import java.util.concurrent.ConcurrentHashMap;
 
 import javax.servlet.http.HttpServletRequest;
 
 import java.util.ArrayList;
 import java.util.Enumeration;
 
-import org.json.JSONObject;
 
 // import org.apache.commons.math.stat.descriptive.SummaryStatistics;
 
@@ -666,77 +662,9 @@ public int getNumUnderway() {
     }
 
     public void resetMatchGraphWithInitialCapacity(int initialCapacity) {
-        matchGraph = null;
+        // Single assignment: never expose a null matchGraph to concurrent mutators
+        // (servlet add/remove) during a rebuild -- the old null-then-new sequence
+        // had an NPE window. (#1608)
         matchGraph = new ConcurrentHashMap<String, EncounterLite>(initialCapacity);
     }
-
-    private static final String CACHE_FILEPATH = "WEB-INF/MatchGraphCache.json";
-
-    public static String getCacheFilePath(String dataDir) {
-        return dataDir + "/" + CACHE_FILEPATH;
-    }
-
-    /**
-     * Serialize the matchGraph to a JSONObject for disk caching.
-     */
-    public static JSONObject cacheToJSONObject() {
-        JSONObject root = new JSONObject();
-        root.put("timestamp", System.currentTimeMillis());
-        JSONObject entries = new JSONObject();
-        for (ConcurrentHashMap.Entry<String, EncounterLite> entry : matchGraph.entrySet()) {
-            entries.put(entry.getKey(), entry.getValue().toJSONObject());
-        }
-        root.put("matchGraph", entries);
-        root.put("count", matchGraph.size());
-        return root;
-    }
-
-    /**
-     * Write the matchGraph cache to disk as JSON.
-     */
-    public static void cacheWrite(String filepath) throws IOException {
-        long t = System.currentTimeMillis();
-        System.out.println("INFO: GridManager.cacheWrite() writing to " + filepath);
-        Util.writeToFile(cacheToJSONObject().toString(), filepath);
-        System.out.println("INFO: GridManager.cacheWrite() complete with " + matchGraph.size() +
-            " entries in " + (System.currentTimeMillis() - t) + "ms");
-    }
-
-    /**
-     * Read the matchGraph cache from disk JSON.
-     * Returns true if the cache was loaded successfully, false otherwise.
-     */
-    public static boolean cacheRead(String filepath) throws IOException {
-        long t = System.currentTimeMillis();
-        matchGraphReady = false;
-        String content = Util.readFromFile(filepath);
-        JSONObject root = Util.stringToJSONObject(content);
-        if (root == null) {
-            System.out.println("ERROR: GridManager.cacheRead() could not parse " + filepath);
-            return false;
-        }
-        System.out.println("INFO: GridManager.cacheRead() from " + filepath +
-            " timestamp=" + root.optLong("timestamp"));
-
-        JSONObject entries = root.optJSONObject("matchGraph");
-        if (entries == null || entries.length() < 1) {
-            System.out.println("ERROR: GridManager.cacheRead() empty matchGraph in " + filepath);
-            return false;
-        }
-
-        matchGraph = new ConcurrentHashMap<String, EncounterLite>(entries.length());
-        Iterator<String> keys = entries.keys();
-        while (keys.hasNext()) {
-            String key = keys.next();
-            JSONObject elJson = entries.optJSONObject(key);
-            if (elJson == null) continue;
-            EncounterLite el = EncounterLite.fromJSONObject(elJson);
-            matchGraph.put(key, el);
-        }
-        resetPatternCounts();
-        matchGraphReady = true;
-        System.out.println("INFO: GridManager.cacheRead() complete with " + matchGraph.size() +
-            " entries in " + (System.currentTimeMillis() - t) + "ms");
-        return true;
-    }
 }

src/main/java/org/ecocean/grid/MatchGraphCreationThread.java

@@ -74,6 +74,14 @@ public void createThem() {
 
             for (int i = 0; i < numEncs; i++) {
                 Encounter enc = myShepherd.getEncounter(encNumbers.get(i));
+                // getEncounter() returns null if the encounter was deleted after the
+                // id snapshot above (a concurrent EncounterDelete during the rebuild)
+                // or otherwise could not be fetched. Skip it rather than NPE-aborting
+                // the whole rebuild. A genuine exception while building the
+                // EncounterLite below is NOT swallowed: it propagates to the outer
+                // catch, which aborts the rebuild WITHOUT flipping matchGraphReady to
+                // true, so we never publish a silently-incomplete graph. (#1608)
+                if (enc == null) continue;
                 if (((enc.getRightSpots() != null) && (enc.getRightSpots().size() > 0)) ||
                     ((enc.getSpots() != null) && (enc.getSpots().size() > 0))) {
                     EncounterLite el = new EncounterLite(enc);

src/main/java/org/ecocean/servlet/EncounterDelete.java

@@ -56,6 +56,22 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
         PrintWriter out = response.getWriter();
         boolean locked = false;
 
+        // Refuse deletion while the match graph is rebuilding from the DB (only
+        // for ~15 min after a restart on spot-matching installs). A concurrent
+        // deletion during the rebuild could be undone by the rebuild's stale read
+        // and resurrect the encounter as a match candidate. (#1608)
+        if (CommonConfiguration.useSpotPatternRecognition(context) &&
+            !GridManager.isMatchGraphReady()) {
+            myShepherd.closeDBTransaction(); // PM opened in ctor; no tx begun yet
+            out.println(ServletUtilities.getHeader(request));
+            out.println(
+                "<strong>Please wait:</strong> the match graph is rebuilding. Please try deleting this encounter again in a few minutes.");
+            out.println(ServletUtilities.getFooter(context));
+            response.setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
+            out.close();
+            return;
+        }
+
         // setup data dir
         String rootWebappPath = getServletContext().getRealPath("/");
         File webappsDir = new File(rootWebappPath).getParentFile();
@@ -171,11 +187,13 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
                     enc2trash.setSkipAutoIndexing(false);
                     myShepherd.throwAwayEncounter(enc2trash);
 
-                    // remove from grid too
-                    GridManager gm = GridManagerFactory.getGridManager();
-                    gm.removeMatchGraphEntry(request.getParameter("number"));
-
-                    myShepherd.commitDBTransaction();
+                    // Only drop the match-graph entry once the deletion is confirmed
+                    // committed, so a swallowed commit failure can't remove a
+                    // still-present encounter from the candidate set. (#1608)
+                    if (myShepherd.commitDBTransactionWithStatus()) {
+                        GridManager gm = GridManagerFactory.getGridManager();
+                        gm.removeMatchGraphEntry(request.getParameter("number"));
+                    }
 
                     // log it
                     log.info("Click to restore deleted encounter: <a href=\"" +

src/main/java/org/ecocean/servlet/EncounterRemoveSpots.java

@@ -44,6 +44,21 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
         boolean locked = false;
         boolean isOwner = true;
         GridManager gm = GridManagerFactory.getGridManager();
+        // Refuse spot removal while the match graph is rebuilding from the DB
+        // (only happens for ~15 min after a restart on spot-matching installs).
+        // A concurrent removal during the rebuild could be overwritten by the
+        // rebuild's stale read and resurrect the deleted spots. (#1608)
+        if (CommonConfiguration.useSpotPatternRecognition(context) &&
+            !GridManager.isMatchGraphReady()) {
+            myShepherd.closeDBTransaction(); // PM opened in ctor; no tx begun yet
+            out.println(ServletUtilities.getHeader(request));
+            out.println(
+                "<strong>Please wait:</strong> the match graph is rebuilding. Please try removing spot data again in a few minutes.");
+            out.println(ServletUtilities.getFooter(context));
+            response.setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
+            out.close();
+            return;
+        }
         // ConcurrentHashMap<String,EncounterLite> chm= gm.getMatchGraph();
         /*
            if(request.getParameter("number")!=null){
@@ -97,9 +112,27 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
                             "-side spot data.</p>");
                         // despotMe.setNumLeftSpots(0);
                     }
-                    gm.addMatchGraphEntry(request.getParameter("number"),
-                        new EncounterLite(despotMe));
-                    myShepherd.commitDBTransaction();
+                    // Build the lite snapshot while the encounter is still managed,
+                    // then update the in-memory match graph AFTER the DB commit so
+                    // (a) a failed commit can't leave the graph inconsistent with the
+                    // DB and (b) the startup rebuild's DB read can't overwrite this
+                    // change with pre-commit (still-spotted) state. (#1608)
+                    boolean stillHasSpots =
+                        ((despotMe.getSpots() != null) && (despotMe.getSpots().size() > 0)) ||
+                        ((despotMe.getRightSpots() != null) &&
+                        (despotMe.getRightSpots().size() > 0));
+                    EncounterLite updatedLite = stillHasSpots ? new EncounterLite(despotMe) : null;
+                    // Only touch the match graph once the DB change is confirmed
+                    // committed, so a swallowed commit failure can't leave the graph
+                    // inconsistent with the DB. (#1608)
+                    if (myShepherd.commitDBTransactionWithStatus()) {
+                        if (stillHasSpots) {
+                            gm.addMatchGraphEntry(request.getParameter("number"), updatedLite);
+                        } else {
+                            // fully de-spotted: drop it from the candidate set entirely
+                            gm.removeMatchGraphEntry(request.getParameter("number"));
+                        }
+                    }
                 } else {
                     locked = true;
                     myShepherd.rollbackDBTransaction();

src/main/java/org/ecocean/servlet/GrothMatchServlet.java

@@ -126,6 +126,11 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
             if (entry.getKey().equals(encNumber)) continue;
             EncounterLite el = entry.getValue();
 
+            // Skip candidates with no spots on the side being scanned. An encounter
+            // whose spot map was deleted must never surface as a match candidate. (#1608)
+            ArrayList candidateSpots = rightScan ? el.getRightSpots() : el.getSpots();
+            if ((candidateSpots == null) || candidateSpots.isEmpty()) continue;
+
             MatchObject mo = el.getPointsForBestMatch(
                 queryArray, epsilon, R, Sizelim, maxTriangleRotation, C,
                 true, rightScan);

src/main/java/org/ecocean/servlet/SubmitSpotsAndImage.java

@@ -165,12 +165,20 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
 	            enc.setSpots(spots);
 	            enc.setLeftReferenceSpots(refSpots);
 	        }
-	        // reset the entry in the GridManager graph
+	        // Build the lite snapshot while the encounter is still managed, then
+	        // update the in-memory match graph AFTER the DB commit so a failed commit
+	        // can't leave the graph inconsistent with the DB and the startup rebuild
+	        // can't overwrite it with pre-commit state. (#1608)
 	        GridManager gm = GridManagerFactory.getGridManager();
-	        gm.addMatchGraphEntry(encId, new EncounterLite(enc));
-
-	        myShepherd.commitDBTransaction();
+	        EncounterLite updatedLite = new EncounterLite(enc);
+
+	        // Only add to the match graph once the spots are confirmed committed, so
+	        // a swallowed commit failure can't leave a phantom candidate. (#1608)
+	        boolean committed = myShepherd.commitDBTransactionWithStatus();
 	        myShepherd.closeDBTransaction();
+	        if (committed) {
+	            gm.addMatchGraphEntry(encId, updatedLite);
+	        }
 
 	        JSONObject rtn = new JSONObject("{\"success\": true}");
 	        rtn.put("spotAssetId", crMa.getId());

src/main/java/org/ecocean/shepherd/core/Shepherd.java

@@ -3358,6 +3358,30 @@ public void commitDBTransaction() {
         }
     }
 
+    /**
+     * Commit and report whether the commit actually succeeded. Unlike
+     * {@link #commitDBTransaction()} (which swallows commit failures and returns
+     * void), this returns false if the transaction was inactive or the commit
+     * threw. Callers can use this to avoid mutating in-memory state after a commit
+     * that did not durably persist -- e.g. only update the GridManager match graph
+     * once the encounter change is confirmed committed. (#1608)
+     */
+    public boolean commitDBTransactionWithStatus() {
+        try {
+            if ((pm != null) && pm.currentTransaction().isActive()) {
+                pm.currentTransaction().commit();
+                ShepherdState.setShepherdState(action + "_" + shepherdID, "commit");
+                return true;
+            }
+            System.out.println("commitDBTransactionWithStatus: transaction was not active.");
+            return false;
+        } catch (Exception e) {
+            System.out.println("commitDBTransactionWithStatus: commit failed: " + e);
+            e.printStackTrace();
+            return false;
+        }
+    }
+
     /**
      * Since we call these together all over Wildbook
      */