From ac6745a355da2a59bcb4b1ffe9fae73c59985a5e Mon Sep 17 00:00:00 2001 From: Willem Elbers Date: Thu, 3 Oct 2024 11:29:45 +0200 Subject: [PATCH] Fixed issue with hanging resource validation, #230 --- ...lCollectionRegistryReferenceCheckImpl.java | 9 ++-- .../crud/v2/editor/CreateAndEditPanel.java | 39 ++++++++-------- .../editors/references/ReferencesEditor.java | 45 ++++++++++++++----- src/main/resources/log4j2.xml | 2 + src/main/resources/profiles/dev/context.xml | 5 +++ 5 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/VirtualCollectionRegistryReferenceCheckImpl.java b/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/VirtualCollectionRegistryReferenceCheckImpl.java index 9b3070ef..6d458ba5 100644 --- a/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/VirtualCollectionRegistryReferenceCheckImpl.java +++ b/src/main/java/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 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 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/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/CreateAndEditPanel.java b/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/CreateAndEditPanel.java index 292ac83b..480867b9 100644 --- a/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/CreateAndEditPanel.java +++ b/src/main/java/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 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.
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.
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.
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/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/editors/references/ReferencesEditor.java b/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/editors/references/ReferencesEditor.java index 9dbeb379..75f08aff 100644 --- a/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/gui/pages/crud/v2/editor/editors/references/ReferencesEditor.java +++ b/src/main/java/eu/clarin/cmdi/virtualcollectionregistry/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 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 resources) { @@ -133,6 +135,16 @@ public synchronized void addAllResources(List 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 getJobs() { return jobs; @@ -140,7 +152,8 @@ public List getJobs() { 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 index 4d317428..dfea912b 100644 --- a/src/main/resources/log4j2.xml +++ b/src/main/resources/log4j2.xml @@ -28,6 +28,8 @@ --> + + diff --git a/src/main/resources/profiles/dev/context.xml b/src/main/resources/profiles/dev/context.xml index 2338b29b..49548676 100644 --- a/src/main/resources/profiles/dev/context.xml +++ b/src/main/resources/profiles/dev/context.xml @@ -155,4 +155,9 @@ + + +