HIPPLUG-1329 [Forward port 3.1] make sure every worker has its own urlset

The Urlset object should have to think about synchronization. It is the code
that uses the Urlset that shouldn't expect the Urlset to be thread-safe (just
like a HashSet isn't thread-safe. It make much more sense to not give all the
workers a reference to the same urlset object but all have their own

A seperate note is that I really think the worker should not extend from
Thread and a lot of the complex logic can be replaced by a simple executer service

(cherry picked from commit 031750fa)

component/src/main/java/org/onehippo/forge/sitemap/components/model/Urlset.java

@@ -62,8 +62,6 @@ public class Urlset {
 
     public static final int MAX_SUPPORTED_URLS_PER_FILE = 50000;
 
-    private final Object lock = new Object();
-
     private List<Url> urls;
 
     public Urlset() {
@@ -108,10 +106,8 @@ public class Urlset {
      * @param url the Url to add
      */
     public void addUrlThatDoesntExistInTheListYet(Url url) {
-        synchronized (lock) {
-            if (!urls.contains(url)) {
-                urls.add(url);
-            }
+        if (!urls.contains(url)) {
+            urls.add(url);
         }
     }
 

component/src/main/java/org/onehippo/forge/sitemap/generator/SitemapGenerator.java

@@ -226,9 +226,8 @@ public class SitemapGenerator {
      * @return The Urlset containing the urls of the sitemap items
      */
     public Urlset createUrlSetBasedOnHstSiteMap() {
-        Urlset urlset = new Urlset();
 
-        createAndStartWorkers(urlset);
+        createAndStartWorkers();
 
         fillInitialWorkQueue();
 
@@ -245,9 +244,22 @@ public class SitemapGenerator {
             throw new IllegalStateException("Error occurred while trying to generate the site map", lastWorkerException);
         }
 
+        Urlset urlset = combineUrlSetsFromWorkers();
+
         return urlset;
     }
 
+    private Urlset combineUrlSetsFromWorkers() {
+        final Urlset combinedUrlset = new Urlset();
+        for (SitemapGeneratorWorker worker : workers) {
+            for (Url url : worker.getUrlset().getUrls()) {
+                combinedUrlset.addUrlThatDoesntExistInTheListYet(url);
+            }
+        }
+        return combinedUrlset;
+    }
+
+
     private void stopWorkers() {
         for (SitemapGeneratorWorker worker : workers) {
             worker.interrupt();
@@ -279,12 +291,12 @@ public class SitemapGenerator {
         }
     }
 
-    private List<SitemapGeneratorWorker> createAndStartWorkers(Urlset urlset) {
+    private List<SitemapGeneratorWorker> createAndStartWorkers() {
         // Initialize the workers
         workers = new ArrayList<SitemapGeneratorWorker>();
         for (int i = 0 ; i < amountOfWorkers; i++) {
             SitemapGeneratorWorker worker = new SitemapGeneratorWorker(
-                    this, mount, urlset, requestContext,
+                    this, mount, requestContext,
                     objectConverter, urlInformationProvider
             );
             worker.start();

component/src/main/java/org/onehippo/forge/sitemap/generator/SitemapGeneratorWorker.java

 /*
- * Copyright 2012-2013 Hippo B.V. (http://www.onehippo.com)
+ * Copyright 2012-2016 Hippo B.V. (http://www.onehippo.com)
  * 
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -99,12 +99,11 @@ public class SitemapGeneratorWorker extends Thread {
      * Constructor.
      *
      * @param generator              the Sitemap generator
-     * @param urlset                 the Urlset which contains all the Urls
      * @param requestContext         the Hst Request context
      * @param objectConverter        the Object converter converts any kind of beans into JCR nodes & properties
      * @param urlInformationProvider the Url information provider
      */
-    public SitemapGeneratorWorker(final SitemapGenerator generator, final Mount mount, final Urlset urlset,
+    public SitemapGeneratorWorker(final SitemapGenerator generator, final Mount mount,
                                   final HstRequestContext requestContext, final ObjectConverter objectConverter,
                                   final UrlInformationProvider urlInformationProvider) {
 
@@ -116,7 +115,7 @@ public class SitemapGeneratorWorker extends Thread {
 
         this.generator = generator;
         this.mount = mount;
-        this.urlset = urlset;
+        this.urlset = new Urlset();
         this.objectConverter = objectConverter;
         this.requestContext = requestContext;
         this.urlInformationProvider = urlInformationProvider;
@@ -131,6 +130,10 @@ public class SitemapGeneratorWorker extends Thread {
         }
     }
 
+    public Urlset getUrlset() {
+        return urlset;
+    }
+
     /**
      * Run method for Threads.
      */