Fixed issue with hanging resource validation, #230

clarin-eric · Oct 3, 2024 · ac6745a · ac6745a
1 parent c61adac
commit ac6745a
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 34 deletions.
diff --git a/...eu/clarin/cmdi/virtualcollectionregistry/VirtualCollectionRegistryReferenceCheckImpl.java b/...eu/clarin/cmdi/virtualcollectionregistry/VirtualCollectionRegistryReferenceCheckImpl.java
@@ -44,6 +44,8 @@ public class VirtualCollectionRegistryReferenceCheckImpl implements VirtualColle
     @Autowired
     private DataStore datastore; //TODO: replace with Spring managed EM?
 
+    private final ReferenceValidator validator = new ReferenceValidator();
+
     @Override
     public void perform(long now) {
         logger.debug("{}", name);        
@@ -53,7 +55,7 @@ public void perform(long now) {
             TypedQuery<VirtualCollection> q
                     = em.createNamedQuery("VirtualCollection.findAllPublic", VirtualCollection.class);
             q.setLockMode(LockModeType.PESSIMISTIC_WRITE);
-            for (VirtualCollection vc : q.getResultList()) {
+            for (VirtualCollection vc : q.getResultList()) {                
                 checkValidityOfReferences(vc);                 
             }
             em.getTransaction().commit();
@@ -72,8 +74,7 @@ public void perform(long now) {
      * @return 
      */
     private ReferenceValidator checkValidityOfReferences(VirtualCollection vc) {
-        ReferenceValidator validator = new ReferenceValidator();
-
+        logger.debug("Validing references for collection with id = {}", vc.getId());
 
         for(Resource resource : vc.getResources()) {
             final Validatable<String> validatable = 
@@ -84,6 +85,8 @@ private ReferenceValidator checkValidityOfReferences(VirtualCollection vc) {
                     //TODO: Does this actually print anything meaningful?
                     logger.error("[RESOURCE ERROR] vc={}, error={}", vc.getId(), error.toString());
                 }
+            } else {
+                logger.debug("Successfully validated reference id={}, url={} for collection with id = {}",resource.getId(), resource.getRef(), vc.getId());
             }
         }
         return validator;

diff --git a/...eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/CreateAndEditPanel.java b/...eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/CreateAndEditPanel.java
@@ -17,6 +17,7 @@
 import eu.clarin.cmdi.virtualcollectionregistry.model.GeneratedBy;
 import eu.clarin.cmdi.virtualcollectionregistry.model.GeneratedByQuery;
 import eu.clarin.cmdi.virtualcollectionregistry.model.VirtualCollection;
+import java.security.Principal;
 import org.apache.wicket.AttributeModifier;
 import org.apache.wicket.Component;
 import org.apache.wicket.ajax.AjaxRequestTarget;
@@ -276,8 +277,17 @@ public void onClick(Optional<AjaxRequestTarget> target) {
                 if(validate()) {
                     mdlErrorMessage.setObject("");
                     lblErrorMessage.setVisible(false);
-                    persist(target.get());
-                    reset();
+                    //Make sure we have a principal, otherwise fail early
+                    Principal principal = getPrincipal() ;
+                    if(principal == null) {
+                        logger.warn("Collection persist attempt with null principal (expired session?).");
+                        mdlErrorMessage.setObject("Save cancelled.<br />Session seemed to have expired.");
+                        lblErrorMessage.setVisible(true);                       
+                    } else {
+                        VirtualCollection newCollection = persist(target.get());
+                        reset();
+                        fireEvent(new AbstractEvent<>(EventType.SAVE, principal, newCollection, target.isEmpty() ? null : target.get()));
+                    }
                 } else {
                     logger.info("Failed to validate");
                     mdlErrorMessage.setObject("Collection failed to validate.<br />Please fix all issues and try again.");
@@ -448,6 +458,7 @@ public void editCollection(VirtualCollection c) {
     }
 
     private void reset() {
+        logger.debug("Resetting editor panel");
         this.originalCollection = null;
         nameModel.setObject("");
         descriptionModel.setObject("");
@@ -491,17 +502,12 @@ private boolean validate() {
         return valid;
     }
 
-    private void persist(final AjaxRequestTarget target) {
-        //Make sure we have a principal, otherwise fail early
+    private Principal getPrincipal() {
         ApplicationSession session = (ApplicationSession)getSession();
-        if(session.getPrincipal() == null) {
-            logger.warn("Collection persist attempt with null principal (expired session?).");
-            mdlErrorMessage.setObject("Save cancelled.<br />Session seemed to have expired.");
-            lblErrorMessage.setVisible(true);
-            target.add(this);
-            return;
-        }
-
+        return session.getPrincipal();
+    }
+
+    private VirtualCollection persist(final AjaxRequestTarget target) {
         VirtualCollection newCollection = new VirtualCollection();
         newCollection.setCreationDate(new Date()); // FIXME: get date from GUI?
 
@@ -542,12 +548,7 @@ private void persist(final AjaxRequestTarget target) {
             genBy.setQuery(new GeneratedByQuery(intQueryProfile.getObject(), intQueryParameters.getObject()));
             newCollection.setGeneratedBy(genBy);
         }
-
-        fireEvent(
-                new AbstractEvent<>(
-                        EventType.SAVE,
-                        session.getPrincipal(),
-                        newCollection,
-                        target));
+
+        return newCollection;
     }
 }
diff --git a/...rtualcollectionregistry/gui/pages/crud/v2/editor/editors/references/ReferencesEditor.java b/...rtualcollectionregistry/gui/pages/crud/v2/editor/editors/references/ReferencesEditor.java
@@ -96,6 +96,8 @@ private void startWorker() {
                 } else {
                     logger.debug("Reference validation worker thread already running");
                 }
+            } else {
+                logger.debug("vcrConfig.isHttpReferenceScanningEnabled() = {}", vcrConfig.isHttpReferenceScanningEnabled());
             }
         }
 
@@ -105,6 +107,13 @@ private void stop() {
             }
         }
 
+        private State getResourceInitialState() {
+            if(vcrConfig.isHttpReferenceScanningEnabled()) {
+                return State.INITIALIZED;
+            }
+            return State.DONE;
+        }
+
         public synchronized  void add(ReferenceJob job) {
             this.jobs.add(job);
             startWorker();
@@ -113,18 +122,11 @@ public synchronized  void add(ReferenceJob job) {
         public synchronized void addAll(List<ReferenceJob> jobs) {
             this.jobs.addAll(jobs);
             startWorker();
-        }
+        }        
 
-        private State getResourceInitialState() {
-            if(vcrConfig.isHttpReferenceScanningEnabled()) {
-                return State.INITIALIZED;
-            }
-            return State.DONE;
-        }
-
-        public synchronized  void addResource(Resource r) {
+        public synchronized  void addResource(Resource r) {            
             this.jobs.add(new ReferenceJob(r, getResourceInitialState()));
-            startWorker();
+            startWorker();            
         }
 
         public synchronized void addAllResources(List<Resource> resources) {
@@ -133,14 +135,25 @@ public synchronized void addAllResources(List<Resource> resources) {
             }
             startWorker();
         }
+
+        public synchronized void rescanResource(int index) {
+            logger.debug("Rescanning resource with index={}", index);
+            if(index >= 0 && index < jobs.size()) {
+                jobs.get(edit_index).setState(getResourceInitialState());
+                startWorker();
+            } else {
+                logger.debug("Rescan index ({}) was out of bounds", index);
+            }
+        }
 
         public List<ReferenceJob> getJobs() {
             return jobs;
         }
 
         public synchronized void updateJobState(int index, State state) {
             jobs.get(index).setState(state);
-            logger.trace("job idx={}, ref={}, state={}", index, jobs.get(index).getReference().getRef(), jobs.get(index).getState());
+            logger.trace("job idx={}, ref={}, state={}", 
+                    index, jobs.get(index).getReference().getRef(), jobs.get(index).getState());
         }
 
         public boolean isEmpty() {
@@ -286,7 +299,8 @@ public void handleEvent(final Event event) {
             @Override
             public void handleSaveEvent(AjaxRequestTarget target) {
                 //Reset state so this reference is rescanned
-                jobs.get(edit_index).setState(State.INITIALIZED);
+                jobs.rescanResource(edit_index);
+                //jobs.get(edit_index).setState(State.INITIALIZED);
                 edit_index = -1;
                 editor.setVisible(false);
                 listview.setVisible(true);
@@ -515,9 +529,11 @@ private boolean handlePid(String value) {
     }
 
     public void reset() {
+        logger.debug("Resetting references editor panel");
         editor.setVisible(false);
         editor.reset();
         jobs.clear();
+        jobs.stop();
         lblNoReferences.setVisible(jobs.isEmpty());
         listview.setVisible(!jobs.isEmpty());
     }
@@ -605,6 +621,7 @@ public boolean isRunning() {
 
         @Override
         public void run() {
+            logger.debug("Worker run()");
             running = true;
             while(running) {
                 try {
@@ -614,8 +631,12 @@ public void run() {
                 }
 
                 synchronized(this) {
+                    if(mgr.size() <= 0) {
+                        logger.debug("#jobs: {}", mgr.size());
+                    }
                     for(int i = 0; i < mgr.size(); i++) {
                         ReferenceJob job = mgr.get(i);
+                        logger.debug("Job id={}, url={}, state={}", job.ref.getId(), job.ref.getRef(), job.getState());
                         if(job.getState() == State.INITIALIZED) {
                             mgr.updateJobState(i, State.ANALYZING);
                             logger.debug("Starting. Job ref={}, state = {}",job.getReference().getRef(), job.getState());

diff --git a/src/main/resources/log4j2.xml b/src/main/resources/log4j2.xml
@@ -28,6 +28,8 @@
         <Logger name="eu.clarin.cmdi.virtualcollectionregistry.gui.pages.crud.v2.editor.CreateAndEditPanel" level="INFO" />    
         <Logger name="eu.clarin.cmdi.virtualcollectionregistry.gui.pages.crud.v2.editor.editors.references.ReferencesEditor" level="INFO" />
         -->
+        <Logger name="eu.clarin.cmdi.virtualcollectionregistry.gui.pages.crud.v2.editor.editors.references" level="DEBUG" />
+
         <Logger name="eu.clarin.cmdi.virtualcollectionregistry.gui.pages.auth" level="INFO" />
         <Logger name="eu.clarin.cmdi.virtualcollectionregistry.gui.CharacterEncodingFilter" level="INFO" />
         <Logger name="eu.clarin.cmdi.virtualcollectionregistry.VirtualCollectionRegistryMaintenanceImpl" level="INFO" />

diff --git a/src/main/resources/profiles/dev/context.xml b/src/main/resources/profiles/dev/context.xml
@@ -155,4 +155,9 @@
     <Parameter name="eu.clarin.cmdi.vcr.forking.enabled"
                value="true"
                override="false" />
+
+    <Parameter name="eu.clarin.cmdi.vcr.reference_scanning.enabled"
+               value="true"
+               override="false" />  
+
 </Context>