[Geoserver-devel] [jira] (GEOS-5074) DefaultCatalogFacade's encapsulation of styles broken, and risks concurrent modification exceptions on styles and layergroups

Gabriel Roldán created GEOS-5074:
------------------------------------

             Summary: DefaultCatalogFacade's encapsulation of styles broken, and risks concurrent modification exceptions on styles and layergroups
                 Key: GEOS-5074
                 URL: https://jira.codehaus.org/browse/GEOS-5074
             Project: GeoServer
          Issue Type: Bug
          Components: Configuration
    Affects Versions: 2.2-beta1
            Reporter: Gabriel Roldán
            Assignee: Gabriel Roldán
             Fix For: 2.2-beta2

Solution is to use {{CopyOnWriteArrayList}} instead of plain {{ArrayList}} to maintain the internal list of styles and layer groups, as it's done for other resources, and to return a safe copy of styles in {{getStyles()}}

Patch:

{code}
--- a/src/main/src/main/java/org/geoserver/catalog/impl/DefaultCatalogFacade.java
+++ b/src/main/src/main/java/org/geoserver/catalog/impl/DefaultCatalogFacade.java
@@ -112,12 +112,12 @@ public class DefaultCatalogFacade implements CatalogFacade {
     /**
      * layer groups
      */
- protected List<LayerGroupInfo> layerGroups = new ArrayList<LayerGroupInfo>();
+ protected List<LayerGroupInfo> layerGroups = new CopyOnWriteArrayList<LayerGroupInfo>();

     /**
      * styles
      */
- protected List<StyleInfo> styles = new ArrayList();
+ protected List<StyleInfo> styles = new CopyOnWriteArrayList<StyleInfo>();

     /**
      * the catalog
@@ -894,9 +894,9 @@ public class DefaultCatalogFacade implements CatalogFacade {
         }
         return null;
     }
-
- public List getStyles() {
- return ModificationProxy.createList(styles,StyleInfo.class);
+
+ public List<StyleInfo> getStyles() {
+ return ModificationProxy.createList(new ArrayList<StyleInfo>(styles), StyleInfo.class);
     }

     public List<StyleInfo> getStylesByWorkspace(WorkspaceInfo workspace) {
diff --git a/src/main/src/test/java/org/geoserver/catalog/impl/CatalogImplTest.java b/src/main/src/test/java/org/geoserver/catalog/impl/CatalogImplTest.java
index 7a2410a..de4ae1a 100644
--- a/src/main/src/test/java/org/geoserver/catalog/impl/CatalogImplTest.java
+++ b/src/main/src/test/java/org/geoserver/catalog/impl/CatalogImplTest.java
@@ -4,6 +4,7 @@ import java.lang.reflect.Proxy;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashSet;
import java.util.List;
import java.util.concurrent.CountDownLatch;

@@ -1168,7 +1169,7 @@ public class CatalogImplTest extends TestCase {
         StyleInfo s2 = catalog.getFactory().createStyle();
         s2.setName(s.getName());
         s2.setFilename(s.getFilename());
-
+
         try {
             catalog.add(s2);
             fail("Shoudl have failed with existing global style with same name");
@@ -1176,10 +1177,14 @@ public class CatalogImplTest extends TestCase {
         catch(IllegalArgumentException expected) {
         }

+ List<StyleInfo> currStyles = catalog.getStyles();
+
         //should pass after setting workspace
         s2.setWorkspace(ws);
         catalog.add(s2);
{code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://jira.codehaus.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira