Fix critical bugs: NPE safety, resource leak, busy-wait

- GraphMatching: null-safe getAtomByID(), guard removeAtom(null) calls
- Reactor: use "-1".equalsIgnoreCase(atom.getID()) pattern for null safety
  across all atom ID comparisons (lines 416-910)
- Reactor: null-safe getContainerAtomByID()
- CallableAtomMappingTool: replace busy-wait spin loop with awaitTermination
- Annotator: fix resource leak in writeSimilarityMatrix() using
  try-with-resources (was leaking FileWriter on exceptions)
- Helper: fix corrupted Javadoc in displayBlankLines()

All 40 tests pass.

Author: Syed Asad Rahman <asad.rahman@bioinceptionlabs.com>

Author: asad

src/main/java/uk/ac/ebi/aamtool/Annotator.java

@@ -186,58 +186,33 @@ private static void writeSimilarityMatrix(List<SimilarityResult> results, String
         File rcMatrix = new File(rootPath, jobID + "_Reaction_Centre" + ".mat");
         File stMatrix = new File(rootPath, jobID + "_Structure_Similarity" + ".mat");
 
-        FileWriter writerBC = new FileWriter(bcMatrix);
-        BufferedWriter bufferedWriterBC = new BufferedWriter(writerBC);
-
-        FileWriter writerRC = new FileWriter(rcMatrix);
-        BufferedWriter bufferedWriterRC = new BufferedWriter(writerRC);
-
-        FileWriter writerST = new FileWriter(stMatrix);
-        BufferedWriter bufferedWriterST = new BufferedWriter(writerST);
-
-        bufferedWriterBC.newLine();
-        try {
+        try (BufferedWriter bufferedWriterBC = new BufferedWriter(new FileWriter(bcMatrix))) {
+            bufferedWriterBC.newLine();
             for (SimilarityResult s : results) {
-                if (s.getSimilarityReactions().containsKey("BC")) {
-                    bufferedWriterBC.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + s.getSimilarityReactions().get("BC"));
-                    bufferedWriterBC.newLine();
-                } else {
-                    bufferedWriterBC.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + "NA");
-                    bufferedWriterBC.newLine();
-                }
+                String val = s.getSimilarityReactions().containsKey("BC")
+                        ? s.getSimilarityReactions().get("BC").toString() : "NA";
+                bufferedWriterBC.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + val);
+                bufferedWriterBC.newLine();
             }
-        } finally {
-            bufferedWriterBC.close();
         }
 
-        try {
+        try (BufferedWriter bufferedWriterRC = new BufferedWriter(new FileWriter(rcMatrix))) {
             for (SimilarityResult s : results) {
-                if (s.getSimilarityReactions().containsKey("RC")) {
-                    bufferedWriterRC.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + s.getSimilarityReactions().get("RC"));
-                    bufferedWriterRC.newLine();
-                } else {
-                    bufferedWriterRC.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + "NA");
-                    bufferedWriterRC.newLine();
-                }
+                String val = s.getSimilarityReactions().containsKey("RC")
+                        ? s.getSimilarityReactions().get("RC").toString() : "NA";
+                bufferedWriterRC.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + val);
+                bufferedWriterRC.newLine();
             }
-        } finally {
-            bufferedWriterRC.close();
         }
 
-        try {
+        try (BufferedWriter bufferedWriterST = new BufferedWriter(new FileWriter(stMatrix))) {
             for (SimilarityResult s : results) {
-                if (s.getSimilarityReactions().containsKey("ST")) {
-                    bufferedWriterST.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + s.getSimilarityReactions().get("ST"));
-                    bufferedWriterST.newLine();
-                } else {
-                    bufferedWriterST.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + "NA");
-                    bufferedWriterST.newLine();
-                }
+                String val = s.getSimilarityReactions().containsKey("ST")
+                        ? s.getSimilarityReactions().get("ST").toString() : "NA";
+                bufferedWriterST.write("\"" + s.getQuery() + "\"" + TAB + "\"" + s.getTarget() + "\"" + TAB + val);
+                bufferedWriterST.newLine();
             }
-        } finally {
-            bufferedWriterST.close();
         }
-
     }
 
     private void printRPAIRPatternAsText(MappingSolution s, StringBuilder sb) throws CloneNotSupportedException {

src/main/java/uk/ac/ebi/aamtool/Helper.java

@@ -80,14 +80,10 @@ protected static void getHeader() {
     }
 
     /**
-     * WreactionWithLayoutite the preactionWithLayoutovided
-     * numbereactionWithLayout of blank lineheaderString to the
-     * preactionWithLayoutovided OutputStreactionWithLayouteam.
+     * Write the provided number of blank lines to the provided OutputStream.
      *
-     * @param numberBlankLines NumbereactionWithLayout of blank lineheaderString
-     * to wreactionWithLayoutite.
-     * @param out OutputStreactionWithLayouteam to which to
-     * wreactionWithLayoutite the blank lineheaderString.
+     * @param numberBlankLines Number of blank lines to write.
+     * @param out OutputStream to which to write the blank lines.
      */
     protected static void displayBlankLines(final int numberBlankLines, final OutputStream out) {
         try {

src/main/java/uk/ac/ebi/reactionblast/mapping/CallableAtomMappingTool.java

@@ -35,6 +35,7 @@
 import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
 import org.openscience.cdk.interfaces.IReaction;
 import org.openscience.cdk.tools.ILoggingTool;
@@ -201,12 +202,7 @@ private synchronized void generateAtomAtomMapping(
                 putSolution(chosen.getAlgorithm(), chosen);
             }
             executor.shutdown();
-            /*
-             Wait until all threads are finish
-             *
-             */
-            while (!executor.isTerminated()) {
-            }
+            executor.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
             LOGGER.debug("======DONE CallableAtomMappingTool=======");
             gc();
         } catch (InterruptedException | ExecutionException e) {

src/main/java/uk/ac/ebi/reactionblast/mapping/Reactor.java

@@ -413,7 +413,7 @@ private synchronized IReaction getMapping(IReaction coreMappedReaction) throws I
             IAtomContainer eMolecule = mappedReaction.getReactants().getAtomContainer(eMol);
             for (int eAtom = 0; eAtom < eMolecule.getAtomCount(); eAtom++) {
                 IAtom atom = mappedReaction.getReactants().getAtomContainer(eMol).getAtom(eAtom);
-                if (!atom.getSymbol().equalsIgnoreCase("H") && atom.getID().equalsIgnoreCase("-1")) {
+                if (!atom.getSymbol().equalsIgnoreCase("H") && "-1".equalsIgnoreCase(atom.getID())) {
                     String atomLabel = Integer.toString(counter);
                     atom.setID(atomLabel);
                     atom.setFlag(MAPPED, false);
@@ -426,7 +426,7 @@ private synchronized IReaction getMapping(IReaction coreMappedReaction) throws I
             IAtomContainer pMolecule = mappedReaction.getProducts().getAtomContainer(pMol);
             for (int pAtom = 0; pAtom < pMolecule.getAtomCount(); pAtom++) {
                 IAtom atom = mappedReaction.getProducts().getAtomContainer(pMol).getAtom(pAtom);
-                if (!atom.getSymbol().equalsIgnoreCase("H") && atom.getID().equalsIgnoreCase("-1")) {
+                if (!atom.getSymbol().equalsIgnoreCase("H") && "-1".equalsIgnoreCase(atom.getID())) {
                     String atomLabel = Integer.toString(counter);
                     atom.setID(atomLabel);
                     atom.setFlag(MAPPED, false);
@@ -442,12 +442,12 @@ private synchronized IReaction getMapping(IReaction coreMappedReaction) throws I
             IAtomContainer eMolecule = mappedReaction.getReactants().getAtomContainer(eMol);
             for (int eAtom = 0; eAtom < eMolecule.getAtomCount(); eAtom++) {
                 IAtom atom = mappedReaction.getReactants().getAtomContainer(eMol).getAtom(eAtom);
-                if (!atom.getSymbol().equalsIgnoreCase("H") && !atom.getID().equalsIgnoreCase("-1")) {
+                if (!atom.getSymbol().equalsIgnoreCase("H") && !"-1".equalsIgnoreCase(atom.getID())) {
                     List<IAtom> eductConnAtoms = eMolecule.getConnectedAtomsList(atom);
                     List<IAtom> productHAtoms = markHAroundCoreAtoms(atom.getID(), mappedReaction.getProducts());
                     for (IAtom eAtomH : eductConnAtoms) {
                         //Collect ummmarked H and map common ones
-                        if (eAtomH.getID().equalsIgnoreCase("-1") && eAtomH.getSymbol().equalsIgnoreCase("H")) {
+                        if ("-1".equalsIgnoreCase(eAtomH.getID()) && eAtomH.getSymbol().equalsIgnoreCase("H")) {
                             if (!productHAtoms.isEmpty()) {
                                 String atomLabel = Integer.toString(counter);
                                 eAtomH.setID(atomLabel);
@@ -818,10 +818,10 @@ private synchronized List<IAtom> markHAroundCoreAtoms(String id, IAtomContainerS
             IAtomContainer pMolecule = molSet.getAtomContainer(pMol);
             for (int pAtom = 0; pAtom < pMolecule.getAtomCount(); pAtom++) {
                 IAtom atom = molSet.getAtomContainer(pMol).getAtom(pAtom);
-                if (!atom.getSymbol().equalsIgnoreCase("H") && !atom.getID().equalsIgnoreCase("-1")) {
-                    if (atom.getID().equalsIgnoreCase(id)) {
+                if (!atom.getSymbol().equalsIgnoreCase("H") && !"-1".equalsIgnoreCase(atom.getID())) {
+                    if (id != null && id.equalsIgnoreCase(atom.getID())) {
                         List<IAtom> conAtoms = pMolecule.getConnectedAtomsList(atom);
-                        conAtoms.stream().filter((atomH) -> (atomH.getID().equalsIgnoreCase("-1") && atomH.getSymbol().equalsIgnoreCase("H"))).forEach((atomH) -> {
+                        conAtoms.stream().filter((atomH) -> ("-1".equalsIgnoreCase(atomH.getID()) && atomH.getSymbol().equalsIgnoreCase("H"))).forEach((atomH) -> {
                             list.add(atomH);
                         });
                     }
@@ -845,7 +845,7 @@ private synchronized List<IAtom> collectUnMappedSingleHAtoms(IAtomContainerSet m
                     IAtom atom = molSet.getAtomContainer(index).getAtom(atomIndex);
                     if (atom.getSymbol().equalsIgnoreCase("H")
                             && !atom.getFlag(MAPPED)
-                            && atom.getID().equalsIgnoreCase("-1")) {
+                            && "-1".equalsIgnoreCase(atom.getID())) {
                         list.add(atom);
                     }
                 }
@@ -867,7 +867,7 @@ private synchronized List<IAtom> collectUnMappedHAtoms(IAtomContainerSet molSet)
                 IAtom atom = molSet.getAtomContainer(index).getAtom(atomIndex);
                 if (atom.getSymbol().equalsIgnoreCase("H")
                         && !atom.getFlag(MAPPED)
-                        && atom.getID().equalsIgnoreCase("-1")) {
+                        && "-1".equalsIgnoreCase(atom.getID())) {
                     list.add(atom);
                 }
             }
@@ -893,7 +893,7 @@ private synchronized int markUnMappedHAtoms(IReaction mappedReaction, int counte
             for (int eAtom = 0; eAtom < eMolecule.getAtomCount(); eAtom++) {
                 IAtom atom = mappedReaction.getReactants().getAtomContainer(eMol).getAtom(eAtom);
                 if (atom.getSymbol().equalsIgnoreCase("H") && !atom.getFlag(MAPPED)
-                        && atom.getID().equalsIgnoreCase("-1")) {
+                        && "-1".equalsIgnoreCase(atom.getID())) {
                     String atomLabel = Integer.toString(localCounter);
                     atom.setFlag(MAPPED, false);
                     atom.setID(atomLabel);
@@ -907,7 +907,7 @@ private synchronized int markUnMappedHAtoms(IReaction mappedReaction, int counte
             for (int pAtom = 0; pAtom < pMolecule.getAtomCount(); pAtom++) {
                 IAtom atom = mappedReaction.getProducts().getAtomContainer(pMol).getAtom(pAtom);
                 if (atom.getSymbol().equalsIgnoreCase("H") && !atom.getFlag(MAPPED)
-                        && atom.getID().equalsIgnoreCase("-1")) {
+                        && "-1".equalsIgnoreCase(atom.getID())) {
                     String atomLabel = Integer.toString(localCounter);
                     atom.setID(atomLabel);
                     atom.setFlag(MAPPED, false);
@@ -940,9 +940,12 @@ private synchronized void setReactionBlastMolMapping(MoleculeMoleculeMapping rea
     }
 
     private synchronized IAtom getContainerAtomByID(IAtomContainerSet products, String mappingID) {
+        if (mappingID == null) {
+            return null;
+        }
         for (IAtomContainer ac : products.atomContainers()) {
             for (IAtom atom : ac.atoms()) {
-                if (atom.getID().equals(mappingID)) {
+                if (mappingID.equals(atom.getID())) {
                     return atom;
                 }
             }

src/main/java/uk/ac/ebi/reactionblast/mapping/graph/GraphMatching.java

@@ -162,15 +162,21 @@ public synchronized int removeMatchedAtomsAndUpdateAAM(IReaction reaction) {
                     IMapping im = SilentChemObjectBuilder.getInstance().newInstance(IMapping.class, eAtom, pAtom);
                     reaction.addMapping(im);
                 }
-                educt.removeAtom(eAtom);
-                product.removeAtom(pAtom);
+                if (eAtom != null) {
+                    educt.removeAtom(eAtom);
+                }
+                if (pAtom != null) {
+                    product.removeAtom(pAtom);
+                }
                 delta = fragmentCount;
             }
         }
 
         for (IAtom atom : educt.atoms()) {
             IAtom matchedAtom = getAtomByID(matchedPart, atom.getID());
-            matchedPart.removeAtom(matchedAtom);
+            if (matchedAtom != null) {
+                matchedPart.removeAtom(matchedAtom);
+            }
         }
 
         LOGGER.debug("After removing Mol Size E: " + educt.getAtomCount()
@@ -197,8 +203,11 @@ public synchronized int removeMatchedAtomsAndUpdateAAM(IReaction reaction) {
     }
 
     private synchronized IAtom getAtomByID(IAtomContainer ac, String ID) {
+        if (ID == null) {
+            return null;
+        }
         for (IAtom atom : ac.atoms()) {
-            if (atom.getID().equalsIgnoreCase(ID)) {
+            if (ID.equalsIgnoreCase(atom.getID())) {
                 return atom;
             }
         }