Close all resources and also insure that ruledataloader doesn't inter…

…fere during periodic runs (#559)

.gitignore

@@ -28,4 +28,4 @@ services/lib
 /ui/src/main/webapp/src/app/tsModels/
 /ui/src/main/webapp/src/app/generated/
 .vscode
-
+services/run_many.sh

addons/web-support/api/src/main/java/org/jboss/windup/web/addons/websupport/rest/graph/GraphCache.java

@@ -27,4 +27,8 @@ public interface GraphCache {
      */
     void closeGraph(Path graphPath);
 
+    /**
+     * Close all open graphs.
+     */
+    void closeAll();
 }

addons/web-support/impl/src/main/java/org/jboss/windup/web/addons/websupport/rest/graph/FileModelResourceImpl.java

@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Logger;
 
 import javax.inject.Singleton;
 import javax.ws.rs.NotFoundException;
@@ -19,6 +20,8 @@
 @Singleton
 public class FileModelResourceImpl extends AbstractGraphResource implements FileModelResource
 {
+    private static Logger LOG = Logger.getLogger(FileModelResourceImpl.class.getCanonicalName());
+
     @Override
     public List<Map<String, Object>> get(Long executionID, String filename)
     {
@@ -36,12 +39,16 @@ public String getSource(Long executionID, Integer vertexID)
             GraphContext context = getGraph(executionID);
             FileService fileService = new FileService(context);
             FileModel fileModel = fileService.getById(vertexID);
+            LOG.info("File loaded: " + fileModel.getElement().id() + " path: " + fileModel.getFilePath());
 
             if (fileModel == null)
                 throw new NotFoundException("FileModel at " + vertexID + " could not be found!");
 
             if (!(fileModel instanceof SourceFileModel))
-                throw new IllegalArgumentException("File " + fileModel.getFileName() + " does not appear to be source code!");
+            {
+                String message = "File " + fileModel + " does not appear to be source code!";
+                throw new IllegalArgumentException(message);
+            }
 
             return IOUtils.toString(fileModel.asInputStream());
         }

addons/web-support/impl/src/main/java/org/jboss/windup/web/addons/websupport/rest/graph/GraphCacheImpl.java

@@ -82,6 +82,11 @@ public void afterStop(Furnace furnace) throws ContainerException
         });
     }
 
+    @Override
+    public void closeAll() {
+        graphContextFactory.closeAll();
+    }
+
     private void cleanup()
     {
         LOG.info("Furnace is shutting down, closing existing graph connections.");
@@ -155,7 +160,7 @@ public void loadGraphEntry(Path graphPath, boolean create)
             return;
 
         LOG.info("Creating cached graph for: " + graphPath);
-        GraphContext graphContext = create ? graphContextFactory.create(graphPath) : graphContextFactory.load(graphPath);
+        GraphContext graphContext = create ? graphContextFactory.create(graphPath, true) : graphContextFactory.load(graphPath);
         GraphCacheEntry graphCacheEntry = new GraphCacheEntry(graphContext);
         this.graphCacheEntryMap.put(graphPath, graphCacheEntry);
     }

addons/web-support/impl/src/main/java/org/jboss/windup/web/addons/websupport/rest/graph/LinkResourceImpl.java

@@ -21,7 +21,10 @@ public List<Map<String, Object>> getLinksToTransformedFiles(Long executionID, In
         FileModel fileModel = fileService.getById(fileModelID);
 
         if (!(fileModel instanceof SourceFileModel))
-            throw new IllegalArgumentException("File must be of type: " + SourceFileModel.class.getSimpleName());
+        {
+            String message = "File " + fileModel + " does not appear to be source code!";
+            throw new IllegalArgumentException(message);
+        }
 
         SourceFileModel sourceFileModel = (SourceFileModel) fileModel;
         return super.frameIterableToResult(executionID, sourceFileModel.getLinksToTransformedFiles(), 0);

services/src/main/java/org/jboss/windup/web/services/RuleDataLoader.java

@@ -14,11 +14,12 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
+import javax.ejb.AccessTimeout;
 import javax.ejb.Schedule;
 import javax.ejb.Singleton;
 import javax.ejb.Startup;
@@ -65,9 +66,6 @@ public class RuleDataLoader
     @PersistenceContext
     private EntityManager entityManager;
 
-    // @Resource
-    // private UserTransaction userTransaction;
-
     @Inject
     private ConfigurationService configurationService;
 
@@ -87,19 +85,36 @@ public class RuleDataLoader
     private RuleFormatterService ruleFormatterService;
 
     @Schedule(hour = "*", minute = "12")
+    @AccessTimeout(value = 3, unit = TimeUnit.MINUTES)
     public void loadPeriodically()
     {
-        Configuration webConfiguration = configurationService.getConfiguration();
-        reloadRuleData(webConfiguration);
-        this.cleanupDanglingRows();
+        LOG.info("Periodic reload of rules data");
+        try
+        {
+            Configuration webConfiguration = configurationService.getConfiguration();
+            reloadRuleData(webConfiguration);
+            this.cleanupDanglingRows();
+        }
+        catch (Throwable t)
+        {
+            LOG.log(Level.WARNING, "Periodic reload failed due to: " + t.getMessage(), t);
+        }
     }
 
+    @AccessTimeout(value = 3, unit = TimeUnit.MINUTES)
     public void configurationUpdated(@Observes Configuration configuration)
     {
         LOG.info("Reloading rule data due to configuration update!");
         reloadRuleData(configuration);
     }
 
+    /*
+     * This prevents timeouts in the case that this is called concurrently.
+     *
+     * The value specified here should be long enough for the previous run to complete.
+     *
+     */
+    @AccessTimeout(value = 3, unit = TimeUnit.MINUTES)
     public void reloadRuleData(Configuration configuration)
     {
         LOG.info("Starting reload of rule data...");
@@ -174,11 +189,11 @@ private void loadRulesForRulesPath(RulesPath rulesPath)
             else
             {
                 final Set<Path> pathsToScan = FileUtils.listFiles(
-                        initialPath.toFile(),
-                        SUPPORTED_RULE_EXTENSIONS, scanRecursively)
-                        .stream()
-                        .map(File::toPath)
-                        .collect(Collectors.toSet());
+                            initialPath.toFile(),
+                            SUPPORTED_RULE_EXTENSIONS, scanRecursively)
+                            .stream()
+                            .map(File::toPath)
+                            .collect(Collectors.toSet());
                 for (Path path : pathsToScan)
                 {
                     loadRules(rulesPath, path);

services/src/test/java/org/jboss/windup/web/services/FurnaceProducerTest.java

@@ -28,7 +28,7 @@ public void testFurnace() throws Exception
         Assert.assertNotNull(furnace);
         Assert.assertNotNull(graphContextFactory);
 
-        try (GraphContext context = graphContextFactory.create())
+        try (GraphContext context = graphContextFactory.create(true))
         {
             Assert.assertNotNull(context);
 

services/src/test/java/org/jboss/windup/web/services/ServerCleanupOnStartup.java

@@ -1,6 +1,7 @@
 package org.jboss.windup.web.services;
 
 import java.nio.file.Path;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import javax.inject.Inject;
@@ -10,6 +11,7 @@
 
 import org.apache.commons.io.FileUtils;
 import org.jboss.windup.web.addons.websupport.WebPathUtil;
+import org.jboss.windup.web.addons.websupport.rest.graph.GraphCache;
 import org.jboss.windup.web.furnaceserviceprovider.FromFurnace;
 
 /**
@@ -20,11 +22,17 @@ public class ServerCleanupOnStartup implements ServletContextListener
 {
     private static Logger LOG = Logger.getLogger(ServerCleanupOnStartup.class.getSimpleName());
 
-    @Inject @FromFurnace
+    @Inject
+    @FromFurnace
     private WebPathUtil webPathUtil;
 
+    @Inject
+    @FromFurnace
+    private GraphCache graphCache;
+
     public ServerCleanupOnStartup()
     {
+        LOG.info("ServerCleanupOnStartup Created!");
     }
 
     @Override
@@ -36,10 +44,20 @@ public void contextInitialized(ServletContextEvent sce)
     @Override
     public void contextDestroyed(ServletContextEvent sce)
     {
+        clearData();
     }
 
     private void clearData()
     {
+        try
+        {
+            graphCache.closeAll();
+        }
+        catch (Throwable t)
+        {
+            LOG.log(Level.WARNING, "Failed at closing previously opened graphs due to: " + t.getMessage(), t);
+        }
+
         Path globalWindupDir = webPathUtil.getGlobalWindupDataPath();
         LOG.info("Clearing windup data from: " + globalWindupDir);
         try

services/src/test/java/org/jboss/windup/web/services/data/WindupExecutionUtil.java

@@ -53,6 +53,7 @@ public WindupExecution executeWindup(MigrationProject project) throws Exception
         System.out.println("Setup Graph test, registered application and ready to start Windup analysis...");
 
         WindupExecution initialExecution = this.windupEndpoint.executeProjectWithContext(context, project.getId());
+        System.out.println("Execution for project: " + project.getId() + " output: " + initialExecution.getOutputPath());
 
         WindupExecution status = this.windupEndpoint.getExecution(initialExecution.getId());
         long beginTime = System.currentTimeMillis();

services/src/test/java/org/jboss/windup/web/services/rest/graph/AbstractGraphResourceTest.java

@@ -60,8 +60,8 @@ public abstract class AbstractGraphResourceTest
     public static WebArchive createDeployment()
     {
         WebArchive war = AbstractTest.createDeployment();
-        war.addAsResource("META-INF/persistence-ondisk.xml", "/META-INF/persistence.xml");
-        war.deleteClass(ServerCleanupOnStartup.class);
+        //war.addAsResource("META-INF/persistence-ondisk.xml", "/META-INF/persistence.xml");
+        //war.deleteClass(ServerCleanupOnStartup.class);
         return war;
     }
 

services/src/test/java/org/jboss/windup/web/services/rest/graph/ClassificationResourceTest.java

@@ -18,13 +18,16 @@
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Logger;
 
 /**
  * @author <a href="mailto:[email protected]">Jesse Sightler</a>
  */
 @RunWith(Arquillian.class)
 public class ClassificationResourceTest extends AbstractGraphResourceTest
 {
+    private static Logger LOG = Logger.getLogger(ClassificationResourceTest.class.getCanonicalName());
+
     @ArquillianResource
     private URL contextPath;
 
@@ -53,6 +56,7 @@ public void testGetClassifications()
         for (Map<String, Object> result : results)
         {
             Integer id = (Integer)result.get(GraphResource.KEY_ID);
+            LOG.info("Getting classifications for: " + id);
             List<Map<String, Object>> classifications = this.classificationResource.getClassifications(executionID, id);
             Assert.assertNotNull(classifications);
             Assert.assertTrue(!classifications.isEmpty());

services/src/test/java/org/jboss/windup/web/services/rest/graph/FileModelResourceTest.java

@@ -3,6 +3,7 @@
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Logger;
 
 import org.jboss.arquillian.container.test.api.RunAsClient;
 import org.jboss.arquillian.junit.Arquillian;
@@ -25,6 +26,7 @@
 @RunWith(Arquillian.class)
 public class FileModelResourceTest extends AbstractGraphResourceTest
 {
+    private static Logger LOG = Logger.getLogger(FileModelResourceTest.class.getCanonicalName());
 
     @ArquillianResource
     private URL contextPath;

services/src/test/java/org/jboss/windup/web/services/rest/graph/HintResourceTest.java

@@ -3,6 +3,7 @@
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Logger;
 
 import org.jboss.arquillian.container.test.api.RunAsClient;
 import org.jboss.arquillian.junit.Arquillian;
@@ -25,6 +26,8 @@
 @RunWith(Arquillian.class)
 public class HintResourceTest extends AbstractGraphResourceTest
 {
+    private static Logger LOG = Logger.getLogger(HintResourceTest.class.getCanonicalName());
+
     @ArquillianResource
     private URL contextPath;
 
@@ -53,6 +56,7 @@ public void testGetHints()
         for (Map<String, Object> result : results)
         {
             Integer id = (Integer) result.get(GraphResource.KEY_ID);
+            LOG.info("Getting hints for id: " + id);
             List<Map<String, Object>> hints = this.hintResource.getHints(executionID, id);
             Assert.assertNotNull(hints);
             Assert.assertTrue(!hints.isEmpty());