Fix harvesting errors not always being reported

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/AbstractHarvester.java

@@ -118,7 +118,12 @@ public abstract class AbstractHarvester<T extends HarvestResult, P extends Abstr
     /**
      * Should we cancel the harvester?
      */
-    protected volatile AtomicBoolean cancelMonitor = new AtomicBoolean(false);
+    protected final AtomicBoolean cancelMonitor = new AtomicBoolean(false);
+
+    /**
+     * Contains all the errors that were thrown during harvesting and that may have caused the harvesting to abort
+     */
+    protected final List<HarvestError> errors = Collections.synchronizedList(new LinkedList<>());
 
     protected ServiceContext context;
 
@@ -145,10 +150,6 @@ public Logger getLogger() {
      * Exception that aborted the harvesting
      */
     private Throwable error;
-    /**
-     * Contains all the warnings and errors that didn't abort the execution, but were thrown during harvesting
-     */
-    private List<HarvestError> errors = Collections.synchronizedList(new LinkedList<>());
     private volatile boolean running = false;
 
     public static AbstractHarvester<?, ?> create(String type, ServiceContext context) throws BadParameterEx, OperationAbortedEx {
@@ -538,7 +539,7 @@ public Status getStatus() {
      * Nested class to handle harvesting with fast indexing.
      */
     public class HarvestWithIndexProcessor extends MetadataIndexerProcessor {
-        Logger logger;
+        private final Logger logger;
 
         public HarvestWithIndexProcessor(DataManager dm, Logger logger) {
             super(dm);
@@ -657,11 +658,6 @@ protected OperResult harvest() {
                         logger.error(t);
                         error = t;
                         errors.add(new HarvestError(context, t));
-                    } finally {
-                        List<HarvestError> harvesterErrors = getErrors();
-                        if (harvesterErrors != null) {
-                            errors.addAll(harvesterErrors);
-                        }
                     }
 
                     long elapsedTime = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() - startTime);
@@ -758,14 +754,8 @@ private Element toElement(List<HarvestError> errors) {
         return res;
     }
 
-    /**
-     * Should be overriden to get a better insight on harvesting
-     * <p/>
-     * Returns the list of exceptions that ocurred during the harvesting but
-     * didn't really stop and abort the harvest.
-     */
     public List<HarvestError> getErrors() {
-        return Collections.synchronizedList(errors);
+        return Collections.unmodifiableList(errors);
     }
 
     public final String getType() {

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/IHarvester.java

@@ -34,11 +34,6 @@
  */
 public interface IHarvester<T extends HarvestResult> {
 
-    /**
-     * Returns all the (important?) exceptions that were thrown during the execution
-     */
-    List<HarvestError> getErrors();
-
     /**
      * Actual harvest function.
      */

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/csw/CswHarvester.java

@@ -91,7 +91,7 @@ protected void storeNodeExtra(CswParams params, String path, String siteId, Stri
      * @throws Exception
      */
     public void doHarvest(Logger log) throws Exception {
-        Harvester h = new Harvester(cancelMonitor, log, context, params);
+        Harvester h = new Harvester(cancelMonitor, log, context, params, errors);
         result = h.harvest(log);
     }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/csw/Harvester.java

@@ -77,24 +77,25 @@ class Harvester implements IHarvester<HarvestResult> {
     private static String CONSTRAINT_LANGUAGE_VERSION = "1.1.0";
 
     //FIXME version should be parametrized
-    private static String GETCAPABILITIES_PARAMETERS = "SERVICE=CSW&REQUEST=GetCapabilities&VERSION=2.0.2";
+    private static final String GETCAPABILITIES_PARAMETERS = "SERVICE=CSW&REQUEST=GetCapabilities&VERSION=2.0.2";
     private final AtomicBoolean cancelMonitor;
 
     private Logger log;
-    private CswParams params;
-    private ServiceContext context;
+    private final CswParams params;
+    private final ServiceContext context;
 
     /**
      * Contains a list of accumulated errors during the executing of this harvest.
      */
-    private List<HarvestError> errors = new LinkedList<HarvestError>();
+    private final List<HarvestError> errors;
 
 
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, CswParams params) {
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, CswParams params, List<HarvestError> errors) {
         this.cancelMonitor = cancelMonitor;
         this.log = log;
         this.context = context;
         this.params = params;
+        this.errors = errors;
     }
 
     public HarvestResult harvest(Logger log) throws Exception {
@@ -734,8 +735,4 @@ private RecordInfo getRecordInfo(Element record) {
         return null;
 
     }
-
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/geoPREST/GeoPRESTHarvester.java

@@ -70,7 +70,7 @@ protected void storeNodeExtra(GeoPRESTParams params, String path,
     //---------------------------------------------------------------------------
 
     public void doHarvest(Logger log) throws Exception {
-        Harvester h = new Harvester(cancelMonitor, log, context, params);
+        Harvester h = new Harvester(cancelMonitor, log, context, params, errors);
         result = h.harvest(log);
     }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/geoPREST/Harvester.java

@@ -81,24 +81,23 @@ class Harvester implements IHarvester<HarvestResult> {
     //--- API methods
     //---
     //---------------------------------------------------------------------------
-    private GeoPRESTParams params;
+    private final GeoPRESTParams params;
 
     //---------------------------------------------------------------------------
-    private ServiceContext context;
+    private final ServiceContext context;
 
     //---------------------------------------------------------------------------
     /**
      * Contains a list of accumulated errors during the executing of this harvest.
      */
-    private List<HarvestError> errors = new LinkedList<HarvestError>();
-
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, GeoPRESTParams params) {
+    private final List<HarvestError> errors;
 
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, GeoPRESTParams params, List<HarvestError> errors) {
         this.cancelMonitor = cancelMonitor;
         this.log = log;
         this.context = context;
         this.params = params;
-
+        this.errors = errors;
     }
 
     public HarvestResult harvest(Logger log) throws Exception {
@@ -307,10 +306,6 @@ protected Date parseDate(String pubDate) throws ParseException {
 
         throw new ParseException("Can't parse date '" + pubDate + "'", 0);
     }
-
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
 }
 
 // =============================================================================

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/geonet/GeonetHarvester.java

@@ -91,7 +91,7 @@ public void addHarvestInfo(Element info, String id, String uuid) {
     }
 
     public void doHarvest(Logger log) throws Exception {
-        Harvester h = new Harvester(cancelMonitor, log, context, params);
+        Harvester h = new Harvester(cancelMonitor, log, context, params, errors);
         result = h.harvest(log);
     }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/geonet/Harvester.java

@@ -71,21 +71,22 @@ class Harvester implements IHarvester<HarvestResult> {
     private final AtomicBoolean cancelMonitor;
     private Logger log;
 
-    private GeonetParams params;
-    private ServiceContext context;
+    private final GeonetParams params;
+    private final ServiceContext context;
 
     /**
      * Contains a list of accumulated errors during the executing of this harvest.
      */
-    private List<HarvestError> errors = new LinkedList<HarvestError>();
+    private final List<HarvestError> errors;
 
     //---------------------------------------------------------------------------
 
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, GeonetParams params) {
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, GeonetParams params, List<HarvestError> errors) {
         this.cancelMonitor = cancelMonitor;
         this.log = log;
         this.context = context;
         this.params = params;
+        this.errors = errors;
     }
 
     public HarvestResult harvest(Logger log) throws Exception {
@@ -426,8 +427,4 @@ private void retrieveLogo(ServiceContext context, final Resources resources, Str
             resources.copyUnknownLogo(context, uuid);
         }
     }
-
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/oaipmh/Harvester.java

@@ -101,13 +101,14 @@ class Harvester extends BaseAligner<OaiPmhParams> implements IHarvester<HarvestR
     /**
      * Contains a list of accumulated errors during the executing of this harvest.
      */
-    private List<HarvestError> errors = new LinkedList<>();
+    private final List<HarvestError> errors;
 
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, OaiPmhParams params) {
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, OaiPmhParams params, List<HarvestError> errors) {
         super(cancelMonitor);
         this.log = log;
         this.context = context;
         this.params = params;
+        this.errors = errors;
 
         result = new HarvestResult();
 
@@ -621,8 +622,4 @@ public void apply(@Nonnull AbstractMetadata entity) {
             result.updatedMetadata++;
         }
     }
-
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/oaipmh/OaiPmhHarvester.java

@@ -77,7 +77,7 @@ protected void storeNodeExtra(OaiPmhParams params, String path,
     //---------------------------------------------------------------------------
 
     public void doHarvest(Logger log) throws Exception {
-        Harvester h = new Harvester(cancelMonitor, log, context, params);
+        Harvester h = new Harvester(cancelMonitor, log, context, params, errors);
         result = h.harvest(log);
     }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/ogcwxs/Harvester.java

@@ -1045,14 +1045,6 @@ private String getBaseUrl(String url) {
         }
     }
 
-    /* (non-Javadoc)
-     * @see org.fao.geonet.kernel.harvest.harvester.IHarvester#getErrors()
-     */
-    @Override
-    public List<HarvestError> getErrors() {
-        return new ArrayList<>();
-    }
-
     private static class WxSLayerRegistry {
         public String uuid;
         public String id;

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/simpleurl/Harvester.java

@@ -78,22 +78,23 @@ class Harvester implements IHarvester<HarvestResult> {
 
     private final AtomicBoolean cancelMonitor;
     private Logger log;
-    private SimpleUrlParams params;
-    private ServiceContext context;
+    private final SimpleUrlParams params;
+    private final ServiceContext context;
 
     @Autowired
     GeonetHttpRequestFactory requestFactory;
 
     /**
      * Contains a list of accumulated errors during the executing of this harvest.
      */
-    private List<HarvestError> errors = new LinkedList<>();
+    private final List<HarvestError> errors;
 
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, SimpleUrlParams params) {
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, SimpleUrlParams params, List<HarvestError> errors) {
         this.cancelMonitor = cancelMonitor;
         this.log = log;
         this.context = context;
         this.params = params;
+        this.errors = errors;
     }
 
     public HarvestResult harvest(Logger log) throws Exception {
@@ -429,8 +430,4 @@ private String retrieveUrl(String url) throws Exception {
     private URI createUrl(String jsonUrl) throws URISyntaxException {
         return new URI(jsonUrl);
     }
-
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/simpleurl/SimpleUrlHarvester.java

@@ -61,7 +61,7 @@ protected void storeNodeExtra(SimpleUrlParams params, String path, String siteId
     }
 
     public void doHarvest(Logger log) throws Exception {
-        Harvester h = new Harvester(cancelMonitor, log, context, params);
+        Harvester h = new Harvester(cancelMonitor, log, context, params, errors);
         result = h.harvest(log);
     }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/thredds/Harvester.java

@@ -191,7 +191,7 @@ class Harvester extends BaseAligner<ThreddsParams> implements IHarvester<Harvest
     private HashSet<String> harvestUris = new HashSet<String>();
     private Map<String, ThreddsService> services = new HashMap<String, Harvester.ThreddsService>();
     private InvCatalogImpl catalog;
-    private List<HarvestError> errors = new LinkedList<HarvestError>();
+    private List<HarvestError> errors;
 
     private LatLonRect globalLatLonBox = null;
     private DateRange globalDateRange = null;
@@ -226,11 +226,12 @@ class Harvester extends BaseAligner<ThreddsParams> implements IHarvester<Harvest
      * @return null
      **/
 
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, ThreddsParams params) {
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, ThreddsParams params, List<HarvestError> errors) {
         super(cancelMonitor);
         this.log = log;
         this.context = context;
         this.params = params;
+        this.errors = errors;
 
         result = new HarvestResult();
 
@@ -320,11 +321,6 @@ public HarvestResult harvest(Logger log) throws Exception {
         return result;
     }
 
-    @Override
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
-
     //---------------------------------------------------------------------------
     //---
     //--- Private methods

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/thredds/ThreddsHarvester.java

@@ -72,7 +72,7 @@ protected void storeNodeExtra(ThreddsParams params, String path,
     //---------------------------------------------------------------------------
 
     public void doHarvest(Logger log) throws Exception {
-        Harvester h = new Harvester(cancelMonitor, log, context, params);
+        Harvester h = new Harvester(cancelMonitor, log, context, params, errors);
         result = h.harvest(log);
     }
 }

harvesters/src/main/java/org/fao/geonet/kernel/harvest/harvester/webdav/Harvester.java

@@ -83,15 +83,16 @@ class Harvester extends BaseAligner<WebDavParams> implements IHarvester<HarvestR
     private UriMapper localUris;
     private HarvestResult result;
     private SchemaManager schemaMan;
-    private List<HarvestError> errors = new LinkedList<>();
+    private List<HarvestError> errors;
     private String processName;
     private Map<String, Object> processParams = new HashMap<>();
 
-    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, WebDavParams params) {
+    public Harvester(AtomicBoolean cancelMonitor, Logger log, ServiceContext context, WebDavParams params, List<HarvestError> errors) {
         super(cancelMonitor);
         this.log = log;
         this.context = context;
         this.params = params;
+        this.errors = errors;
 
         result = new HarvestResult();
         result.addedMetadata = 0;
@@ -291,7 +292,7 @@ private void addMetadata(RemoteFile rf) throws Exception {
         if (StringUtils.isNotEmpty(params.xslfilter)) {
             md = HarvesterUtil.processMetadata(dataMan.getSchema(schema),
                 md, processName, processParams);
-                
+
             schema = dataMan.autodetectSchema(md);
         }
 
@@ -525,10 +526,6 @@ private void updateMetadata(RemoteFile rf, RecordInfo recordInfo, boolean force)
             dataMan.indexMetadata(recordInfo.id, true);
         }
     }
-
-    public List<HarvestError> getErrors() {
-        return errors;
-    }
 }
 
 //=============================================================================