Fix failing gs-gwc tests after TileLayerConfiguration interface changes

- Rewrite mock tests to work with new autosaving config implementation - Support new rename approach - Key by name rather than id
geoserver · Jan 2, 2018 · 9d1260b · 9d1260b
1 parent 9e8d5a2
commit 9d1260b
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 58 deletions.
diff --git a/src/gwc/src/main/java/org/geoserver/gwc/GWC.java b/src/gwc/src/main/java/org/geoserver/gwc/GWC.java
@@ -1631,6 +1631,13 @@ public void save(final TileLayer layer) {
         tld.modify(layer);
     }
 
+    public void rename(String oldTileLayerName, String newTileLayerName) {
+        checkNotNull(oldTileLayerName);
+        checkNotNull(newTileLayerName);
+        log.info("Renaming GeoSeverTileLayer " + oldTileLayerName + " to " + newTileLayerName);
+        tld.rename(oldTileLayerName, newTileLayerName);
+    }
+
     /**
      * Returns the tile layers that refer to the given style, either as the tile layer's
      * {@link GeoServerTileLayer#getStyles() default style} or one of the
@@ -2016,24 +2023,24 @@ public void autoConfigureLayers(List<String> catalogLayerNames, GWCConfig saneCo
      *         is not an instance of {@link LayerInfo} or {@link LayerGroupInfo}
      */
     public boolean hasTileLayer(CatalogInfo source) {
-        final String tileLayerId;
+        final String tileLayerName;
         if (source instanceof ResourceInfo) {
             LayerInfo layerInfo = getCatalog().getLayerByName(
                     ((ResourceInfo) source).prefixedName());
             if (layerInfo == null) {
                 return false;
             }
-            tileLayerId = layerInfo.getId();
+            tileLayerName = tileLayerName(layerInfo);
         } else if (source instanceof LayerInfo) {
-            tileLayerId = ((LayerInfo) source).getId();
+            tileLayerName = tileLayerName((LayerInfo) source);
         } else if (source instanceof LayerGroupInfo) {
-            tileLayerId = ((LayerGroupInfo) source).getId();
+            tileLayerName = tileLayerName((LayerGroupInfo) source);
         } else {
             return false;
         }
         BaseConfiguration configuration;
         try {
-            configuration = tld.getConfiguration(tileLayerId);
+            configuration = tld.getConfiguration(tileLayerName);
         } catch (IllegalArgumentException notFound) {
             return false;
         }

diff --git a/src/gwc/src/main/java/org/geoserver/gwc/layer/CatalogConfiguration.java b/src/gwc/src/main/java/org/geoserver/gwc/layer/CatalogConfiguration.java
@@ -548,6 +548,38 @@ public synchronized void modifyLayer(TileLayer tl) throws NoSuchElementException
         save();
     }
 
+    /**
+     * @see org.geowebcache.config.TileLayerConfiguration#modifyLayer(org.geowebcache.layer.TileLayer)
+     */
+    @Override
+    public synchronized void renameLayer(String oldName, String newName) throws NoSuchElementException {
+        TileLayer tl = getLayer(oldName);
+        checkNotNull(tl, "TileLayer " + oldName+ " not found");
+        checkArgument(canSave(tl), "Can't rename TileLayer of type ", tl.getClass());
+
+        GeoServerTileLayer tileLayer = (GeoServerTileLayer) tl;
+
+        checkNotNull(tileLayer.getInfo(), "GeoServerTileLayerInfo is null");
+        checkNotNull(tileLayer.getInfo().getId(), "id is null");
+        checkNotNull(tileLayer.getInfo().getName(), "name is null");
+
+        final GeoServerTileLayerInfo info = tileLayer.getInfo();
+        lock.acquireWriteLock();
+        try {
+            final String layerId = info.getId();
+            info.setName(newName);
+            // check pendingModifications too to catch unsaved adds
+            boolean exists = pendingModications.containsKey(layerId)
+                    || tileLayerCatalog.exists(layerId);
+            checkArgument(exists, "No GeoServerTileLayer named '" + info.getName() + "' exists");
+            pendingModications.put(layerId, info);
+            layerCache.invalidate(layerId);
+        } finally {
+            lock.releaseWriteLock();
+        }
+        save();
+    }
+
     /**
      * {@link TileLayerDispatcher} is requesting to remove the layer named after {@code layerName}
      * 

diff --git a/src/gwc/src/main/java/org/geoserver/gwc/layer/CatalogLayerEventListener.java b/src/gwc/src/main/java/org/geoserver/gwc/layer/CatalogLayerEventListener.java
@@ -10,10 +10,7 @@
 import static org.geoserver.gwc.GWC.tileLayerName;
 
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -585,14 +582,7 @@ private void renameTileLayer(final GeoServerTileLayerInfo tileLayerInfo, String
 
         final GeoServerTileLayer modifiedTileLayer;
 
-        if (oldTileLayer.getLayerInfo() != null) {
-            LayerInfo layerInfo = oldTileLayer.getLayerInfo();
-            modifiedTileLayer = new GeoServerTileLayer(layerInfo, gridSetBroker, tileLayerInfo);
-        } else {
-            LayerGroupInfo layerGroup = oldTileLayer.getLayerGroupInfo();
-            modifiedTileLayer = new GeoServerTileLayer(layerGroup, gridSetBroker, tileLayerInfo);
-        }
-        mediator.save(modifiedTileLayer);
+        mediator.rename(oldLayerName, newLayerName);
     }
 
     /**

diff --git a/src/gwc/src/test/java/org/geoserver/gwc/layer/CatalogConfigurationTest.java b/src/gwc/src/test/java/org/geoserver/gwc/layer/CatalogConfigurationTest.java
@@ -21,17 +21,9 @@
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import static org.mockito.Mockito.*;
+
+import java.util.*;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
@@ -281,15 +273,27 @@ private void testModifyLayer(GeoServerTileLayer orig) {
 
         assertEquals(orig.getInfo(), config.getLayer(orig.getName()).getInfo());
 
+        //Update mocks
+        when(tileLayerCatalog.save(modified.getInfo())).thenReturn(orig.getInfo());
+
         config.modifyLayer(modified);
 
+        when(tileLayerCatalog.getLayerById(modified.getId())).thenReturn(modified.getInfo());
+
         assertEquals(newState, config.getLayer(orig.getName()).getInfo());
 
         final String origName = orig.getName();
         modified.getInfo().setName("changed");
 
+        when(tileLayerCatalog.save(modified.getInfo())).thenReturn(orig.getInfo());
+
         config.modifyLayer(modified);
 
+        //Update mocks
+        when(tileLayerCatalog.getLayerById(orig.getId())).thenReturn(null);
+        when(tileLayerCatalog.getLayerId(orig.getName())).thenReturn(null);
+        when(tileLayerCatalog.getLayerNames()).thenReturn(Collections.emptySet());
+
         assertNull(config.getLayer(origName));
         assertFalse(config.getLayerNames().contains(origName));
     }
@@ -314,13 +318,30 @@ private void testModifyLayer(GeoServerTileLayer orig) {
 
         assertTrue(config.removeLayer(layerName));
 
+        //Update mocks
+        when(tileLayerCatalog.getLayerByName(layerName)).thenReturn(null);
+        when(tileLayerCatalog.getLayerId(layerName)).thenReturn(null);
+        when(tileLayerCatalog.getLayerNames()).thenReturn(
+                ImmutableSet.of(tileLayerName(layer2), tileLayerName(group1), tileLayerName(group2)));
+        when(tileLayerCatalog.getLayerIds()).thenReturn(
+                ImmutableSet.of(layer2.getId(), group1.getId(), group2.getId()));
+
         assertNull(config.getLayer(layerName));
         assertFalse(config.getLayerNames().contains(layerName));
         assertEquals(initialCount - 1, config.getLayerCount());
 
         layerName = GWC.tileLayerName(group1);
         assertNotNull(config.getLayer(layerName));
         assertTrue(config.removeLayer(layerName));
+
+        //Update mocks
+        when(tileLayerCatalog.getLayerByName(layerName)).thenReturn(null);
+        when(tileLayerCatalog.getLayerId(layerName)).thenReturn(null);
+        when(tileLayerCatalog.getLayerNames()).thenReturn(
+                ImmutableSet.of(tileLayerName(layer2), tileLayerName(group2)));
+        when(tileLayerCatalog.getLayerIds()).thenReturn(
+                ImmutableSet.of(layer2.getId(), group2.getId()));
+
         assertNull(config.getLayer(layerName));
         assertEquals(initialCount - 2, config.getLayerCount());
     }
@@ -336,28 +357,22 @@ private void testModifyLayer(GeoServerTileLayer orig) {
         config.modifyLayer(new GeoServerTileLayer(layer1, gridSetBroker, forceState1));
 
         verify(mockMediator, never()).layerRemoved(anyString());
-        verify(mockMediator, never()).layerRenamed(anyString(), anyString());
-
-        config.save();
-
         verify(tileLayerCatalog, times(1)).save(same(forceState1));
-
-        // and gwc has been instructed on the changes
         verify(mockMediator, times(1)).layerRenamed(eq(layerInfo1.getName()), eq("newName"));
     }
 
     @Test public void testSave() {
-        // add a pending delete
+        // delete layer
         when(tileLayerCatalog.delete(eq(layerInfo2.getId()))).thenReturn(layerInfo2);
         assertTrue(config.removeLayer(layerInfo2.getName()));
 
-        // and a failed one at save
+        // failing delete
         when(tileLayerCatalog.delete(eq(groupInfo1.getId()))).thenReturn(groupInfo1);
         assertTrue(config.removeLayer(groupInfo1.getName()));
         doThrow(new IllegalArgumentException("failedDelete")).when(tileLayerCatalog).delete(
                 eq(group1.getId()));
 
-        // add two pending modifications
+        // modify two layers, one will fail
         GeoServerTileLayerInfo forceState1 = TileLayerInfoUtil.loadOrCreate(layer1, defaults);
         forceState1.setName("newName");
 
@@ -370,9 +385,6 @@ private void testModifyLayer(GeoServerTileLayer orig) {
         doThrow(new IllegalArgumentException("failedSave")).when(tileLayerCatalog).save(
                 eq(forceState2));
 
-        verify(mockMediator, times(1)).layerRemoved(layerInfo2.getName());
-        verify(mockMediator, never()).layerRenamed(anyString(), anyString());
-
         GeoServerTileLayerInfo addedState1 = TileLayerInfoUtil.loadOrCreate(layerWithNoTileLayer,
                 defaults);
         config.addLayer(new GeoServerTileLayer(layerWithNoTileLayer, gridSetBroker, addedState1));
@@ -383,7 +395,6 @@ private void testModifyLayer(GeoServerTileLayer orig) {
                 defaults);
         config.addLayer(new GeoServerTileLayer(groupWithNoTileLayer, gridSetBroker, addedState2));
 
-        config.save();
 
         verify(tileLayerCatalog, times(1)).delete(eq(group1.getId()));
         verify(tileLayerCatalog, times(1)).delete(eq(layer2.getId()));
@@ -392,7 +403,6 @@ private void testModifyLayer(GeoServerTileLayer orig) {
         verify(tileLayerCatalog, times(1)).save(same(addedState1));
         verify(tileLayerCatalog, times(1)).save(same(addedState2));
 
-        // and gwc has been instructed on the changes
         verify(mockMediator, times(1)).layerRemoved(eq(layerInfo2.getName()));
         verify(mockMediator, times(1)).layerRenamed(eq(layerInfo1.getName()), eq("newName"));
         verify(mockMediator, times(1)).layerAdded(eq(addedState1.getName()));

diff --git a/src/gwc/src/test/java/org/geoserver/gwc/layer/CatalogLayerEventListenerTest.java b/src/gwc/src/test/java/org/geoserver/gwc/layer/CatalogLayerEventListenerTest.java
@@ -191,11 +191,9 @@ public void setUp() throws Exception {
 
         listener.handlePostModifyEvent(postModifyEvent);
 
-        ArgumentCaptor<GeoServerTileLayer> captor = ArgumentCaptor
-                .forClass(GeoServerTileLayer.class);
-        verify(mockMediator).save(captor.capture());
+        verify(mockMediator).rename(oldTileLayerName, renamedPrefixedResouceName);
 
-        GeoServerTileLayer saved = captor.getValue();
+        GeoServerTileLayer saved = mockMediator.getTileLayer(mockResourceInfo);
         assertNotNull(saved);
         assertNotNull(saved.getInfo());
         GeoServerTileLayerInfo savedInfo = saved.getInfo();
@@ -271,11 +269,9 @@ public void testCqlFilterChanged() throws Exception {
 
         listener.handlePostModifyEvent(postModifyEvent);
 
-        ArgumentCaptor<GeoServerTileLayer> captor = ArgumentCaptor
-                .forClass(GeoServerTileLayer.class);
-        verify(mockMediator).save(captor.capture());
+        verify(mockMediator).rename(oldGroupName, renamedGroupName);
 
-        GeoServerTileLayer saved = captor.getValue();
+        GeoServerTileLayer saved = mockMediator.getTileLayer(mockLayerGroupInfo);
         assertNotNull(saved);
         assertNotNull(saved.getInfo());
         GeoServerTileLayerInfo savedInfo = saved.getInfo();
@@ -321,11 +317,9 @@ public void testCqlFilterChanged() throws Exception {
 
         listener.handlePostModifyEvent(postModifyEvent);
 
-        ArgumentCaptor<GeoServerTileLayer> captor = ArgumentCaptor
-                .forClass(GeoServerTileLayer.class);
-        verify(mockMediator).save(captor.capture());
+        verify(mockMediator).rename(oldLayerName, prefixedName);
 
-        GeoServerTileLayer saved = captor.getValue();
+        GeoServerTileLayer saved = mockMediator.getTileLayer(mockLayerGroupInfo);
         assertNotNull(saved);
         assertNotNull(saved.getInfo());
         GeoServerTileLayerInfo savedInfo = saved.getInfo();
@@ -369,11 +363,9 @@ public void testCqlFilterChanged() throws Exception {
 
         listener.handlePostModifyEvent(postModifyEvent);
 
-        ArgumentCaptor<GeoServerTileLayer> captor = ArgumentCaptor
-                .forClass(GeoServerTileLayer.class);
-        verify(mockMediator).save(captor.capture());
+        verify(mockMediator).rename(oldPrefixedName, newPrefixedName);
 
-        GeoServerTileLayer saved = captor.getValue();
+        GeoServerTileLayer saved = mockMediator.getTileLayer(mockResourceInfo);
         assertNotNull(saved);
         assertNotNull(saved.getInfo());
         GeoServerTileLayerInfo savedInfo = saved.getInfo();