[GEOS-7403] TransactionPlugin can no longer alter transaction items

src/gwc/src/main/java/org/geoserver/gwc/GWCTransactionListener.java

@@ -17,17 +17,12 @@
 
 import javax.xml.namespace.QName;
 
-import net.opengis.wfs.DeleteElementType;
-import net.opengis.wfs.InsertElementType;
-import net.opengis.wfs.TransactionResponseType;
-import net.opengis.wfs.TransactionType;
-import net.opengis.wfs.UpdateElementType;
+import net.opengis.wfs.*;
 
 import org.eclipse.emf.ecore.EObject;
-import org.geoserver.wfs.TransactionEvent;
-import org.geoserver.wfs.TransactionEventType;
-import org.geoserver.wfs.TransactionPlugin;
-import org.geoserver.wfs.WFSException;
+import org.geoserver.wfs.*;
+import org.geoserver.wfs.request.TransactionRequest;
+import org.geoserver.wfs.request.TransactionResponse;
 import org.geotools.data.simple.SimpleFeatureCollection;
 import org.geotools.geometry.jts.ReferencedEnvelope;
 import org.geotools.geometry.jts.ReferencedEnvelope3D;
@@ -54,7 +49,7 @@
  * @version $Id$
  * 
  */
-public class GWCTransactionListener implements TransactionPlugin {
+public class GWCTransactionListener implements TransactionPlugin2 {
 
     private static Logger log = Logging.getLogger(GWCTransactionListener.class);
 
@@ -72,10 +67,8 @@ public GWCTransactionListener(final GWC gwc) {
     /**
      * Not used, we're interested in the {@link #dataStoreChange} and {@link #afterTransaction}
      * hooks
-     * 
-     * @see org.geoserver.wfs.TransactionPlugin#beforeTransaction(net.opengis.wfs.TransactionType)
      */
-    public TransactionType beforeTransaction(TransactionType request) throws WFSException {
+    public TransactionRequest beforeTransaction(TransactionRequest request) throws WFSException {
         // nothing to do
         return request;
     }
@@ -84,9 +77,9 @@ public TransactionType beforeTransaction(TransactionType request) throws WFSExce
      * Not used, we're interested in the {@link #dataStoreChange} and {@link #afterTransaction}
      * hooks
      * 
-     * @see org.geoserver.wfs.TransactionPlugin#beforeCommit(net.opengis.wfs.TransactionType)
+     * @see org.geoserver.wfs.TransactionPlugin#beforeCommit(net.opengis.wfs.TransactionRequest)
      */
-    public void beforeCommit(TransactionType request) throws WFSException {
+    public void beforeCommit(TransactionRequest request) throws WFSException {
         // nothing to do
     }
 
@@ -96,8 +89,7 @@ public void beforeCommit(TransactionType request) throws WFSException {
      * 
      * @see org.geoserver.wfs.TransactionPlugin#afterTransaction
      */
-    public void afterTransaction(final TransactionType request, TransactionResponseType result,
-            boolean committed) {
+    public void afterTransaction(final TransactionRequest request, TransactionResponse result, boolean committed) {
         if (!committed) {
             return;
         }
@@ -109,7 +101,7 @@ public void afterTransaction(final TransactionType request, TransactionResponseT
         }
     }
 
-    private void afterTransactionInternal(final TransactionType transaction, boolean committed) {
+    private void afterTransactionInternal(final TransactionRequest transaction, boolean committed) {
 
         final Map<String, List<ReferencedEnvelope>> byLayerDirtyRegions = getByLayerDirtyRegions(transaction);
         if (byLayerDirtyRegions.isEmpty()) {
@@ -202,15 +194,16 @@ private void dataStoreChangeInternal(final TransactionEvent event) {
         final ReferencedEnvelope affectedBounds = affectedFeatures.getBounds();
 
         final TransactionType transaction = event.getRequest();
+        TransactionRequest request = TransactionRequest.adapt(transaction);
 
         for (String tileLayerName : affectedTileLayers) {
-            addLayerDirtyRegion(transaction, tileLayerName, affectedBounds);
+            addLayerDirtyRegion(request, tileLayerName, affectedBounds);
         }
     }
 
     @SuppressWarnings("unchecked")
     private Map<String, List<ReferencedEnvelope>> getByLayerDirtyRegions(
-            final TransactionType transaction) {
+            final TransactionRequest transaction) {
 
         final Map<Object, Object> extendedProperties = transaction.getExtendedProperties();
         Map<String, List<ReferencedEnvelope>> byLayerDirtyRegions;
@@ -223,7 +216,7 @@ private Map<String, List<ReferencedEnvelope>> getByLayerDirtyRegions(
         return byLayerDirtyRegions;
     }
 
-    private void addLayerDirtyRegion(final TransactionType transaction, final String tileLayerName,
+    private void addLayerDirtyRegion(final TransactionRequest transaction, final String tileLayerName,
             final ReferencedEnvelope affectedBounds) {
 
         Map<String, List<ReferencedEnvelope>> byLayerDirtyRegions = getByLayerDirtyRegions(transaction);

src/gwc/src/test/java/org/geoserver/gwc/GWCTransactionListenerTest.java

@@ -5,32 +5,13 @@
  */
 package org.geoserver.gwc;
 
-import static junit.framework.Assert.assertNotNull;
-import static junit.framework.Assert.assertSame;
-import static junit.framework.Assert.assertTrue;
-import static junit.framework.Assert.fail;
-import static org.geotools.referencing.crs.DefaultGeographicCRS.WGS84;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.when;
-
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import javax.xml.namespace.QName;
-
+import com.google.common.collect.ImmutableSet;
 import net.opengis.wfs.InsertElementType;
-import net.opengis.wfs.TransactionResponseType;
 import net.opengis.wfs.TransactionType;
-
 import org.geoserver.wfs.TransactionEvent;
 import org.geoserver.wfs.TransactionEventType;
+import org.geoserver.wfs.request.TransactionRequest;
+import org.geoserver.wfs.request.TransactionResponse;
 import org.geotools.data.simple.SimpleFeatureCollection;
 import org.geotools.geometry.jts.ReferencedEnvelope;
 import org.geotools.geometry.jts.ReferencedEnvelope3D;
@@ -39,7 +20,20 @@
 import org.junit.Test;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 
-import com.google.common.collect.ImmutableSet;
+import javax.xml.namespace.QName;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static junit.framework.TestCase.assertTrue;
+import static org.geotools.referencing.crs.DefaultGeographicCRS.WGS84;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.*;
 
 public class GWCTransactionListenerTest {
 
@@ -56,9 +50,9 @@ public void setUp() throws Exception {
     @Test
     public void testNoInteractionsInUnusedMethods() {
 
-        TransactionType request = mock(TransactionType.class);
+        TransactionRequest request = mock(TransactionRequest.class);
 
-        TransactionType returned = listener.beforeTransaction(request);
+        TransactionRequest returned = listener.beforeTransaction(request);
         assertSame(request, returned);
         verifyNoMoreInteractions(request, mediator);
 
@@ -69,8 +63,8 @@ public void testNoInteractionsInUnusedMethods() {
     @Test
     public void testAfterTransactionUncommitted() {
 
-        TransactionType request = mock(TransactionType.class);
-        TransactionResponseType result = mock(TransactionResponseType.class);
+        TransactionRequest request = mock(TransactionRequest.class);
+        TransactionResponse result = mock(TransactionResponse.class);
         boolean committed = false;
 
         listener.afterTransaction(request, result, committed);
@@ -171,8 +165,8 @@ public void testAfterTransactionCompoundCRS() throws Exception {
 
         issueInsert(extendedProperties, transactionBounds);
 
-        TransactionType request = mock(TransactionType.class);
-        TransactionResponseType result = mock(TransactionResponseType.class);
+        TransactionRequest request = mock(TransactionRequest.class);
+        TransactionResponse result = mock(TransactionResponse.class);
         when(request.getExtendedProperties()).thenReturn(extendedProperties);
 
         when(mediator.getDeclaredCrs(anyString())).thenReturn(compoundCrs);
@@ -194,8 +188,8 @@ public void testAfterTransaction() throws Exception {
 
         issueInsert(extendedProperties, affectedBounds2);
 
-        TransactionType request = mock(TransactionType.class);
-        TransactionResponseType result = mock(TransactionResponseType.class);
+        TransactionRequest request = mock(TransactionRequest.class);
+        TransactionResponse result = mock(TransactionResponse.class);
         when(request.getExtendedProperties()).thenReturn(extendedProperties);
 
         when(mediator.getDeclaredCrs(anyString())).thenReturn(WGS84);

src/wfs/src/main/java/org/geoserver/wfs/Transaction.java

@@ -71,9 +71,10 @@ public class Transaction {
 
     /** Geotools2 transaction used for this opperations */
     protected org.geotools.data.Transaction transaction;
-    protected List transactionElementHandlers = new ArrayList();
-    protected List transactionListeners = new ArrayList();
-    protected List transactionPlugins = new ArrayList();
+    protected List<TransactionElementHandler> transactionElementHandlers = new ArrayList<>();
+    protected List<TransactionListener> transactionListeners = new ArrayList<>();
+    protected List<TransactionPlugin> transactionPlugins = new ArrayList<>();
+    protected List<TransactionPlugin2> transactionPlugins2 = new ArrayList<>();
 
     public Transaction(WFSInfo wfs, Catalog catalog, ApplicationContext context) {
         this.wfs = wfs;
@@ -83,10 +84,12 @@ public Transaction(WFSInfo wfs, Catalog catalog, ApplicationContext context) {
         transactionElementHandlers.addAll(GeoServerExtensions.extensions(TransactionElementHandler.class));
         transactionListeners.addAll(GeoServerExtensions.extensions(TransactionListener.class));
         transactionPlugins.addAll(GeoServerExtensions.extensions(TransactionPlugin.class));
+        transactionPlugins2.addAll(GeoServerExtensions.extensions(TransactionPlugin2.class));
         // plugins are listeners too, but I want to make sure they are notified
         // of
         // changes in the same order as the other plugin callbacks
         transactionListeners.removeAll(transactionPlugins);
+        transactionListeners.removeAll(transactionPlugins2);
         // sort plugins according to priority
         Collections.sort(transactionPlugins, new TransactionPluginComparator());
     }
@@ -157,10 +160,7 @@ protected TransactionResponse execute(TransactionRequest request)
 
         // inform plugins we're about to start, and let them eventually
         // alter the request
-        for (Iterator it = transactionPlugins.iterator(); it.hasNext();) {
-            TransactionPlugin tp = (TransactionPlugin) it.next();
-            fireBeforeTransaction(request, tp);
-        }
+        request = fireBeforeTransaction(request);
 
         // setup the transaction listener multiplexer
         TransactionListenerMux multiplexer = new TransactionListenerMux();
@@ -344,11 +344,8 @@ protected TransactionResponse execute(TransactionRequest request)
             if (exception != null) {
                 transaction.rollback();
             } else {
-                // inform plugins we're about to commit
-                for (Iterator it = transactionPlugins.iterator(); it.hasNext();) {
-                    TransactionPlugin tp = (TransactionPlugin) it.next();
-                    fireBeforeCommit(request, tp);
-                }
+                fireBeforeCommit(request);
+
 
                 transaction.commit();
                 committed = true;
@@ -387,10 +384,7 @@ protected TransactionResponse execute(TransactionRequest request)
         }
 
         // inform plugins we're done
-        for (Iterator it = transactionPlugins.iterator(); it.hasNext();) {
-            TransactionPlugin tp = (TransactionPlugin) it.next();
-            fireAfterTransaction(request, result, committed, tp);
-        }
+        fireAfterTransaction(request, result, committed);
 
         //        
         // if ( result.getTransactionResult().getStatus().getPARTIAL() != null )
@@ -444,23 +438,49 @@ protected TransactionResponse execute(TransactionRequest request)
         // response = build;
     }
 
-    void fireAfterTransaction(TransactionRequest request, TransactionResponse result, boolean committed, TransactionPlugin tp) {
+    private TransactionRequest fireBeforeTransaction(TransactionRequest request) {
         TransactionType tx = TransactionRequest.WFS11.unadapt(request);
-        TransactionResponseType tr = TransactionResponse.WFS11.unadapt(result);
-
-        if (tx != null && tr != null) tp.afterTransaction(tx, tr, committed);
+        if (tx != null) {
+            // TransactionPlugin cannot alter transactions since the advent of WFS 2.0, it's left like that and
+            // will be deprecated
+            for (TransactionPlugin tp : transactionPlugins) {
+                tp.beforeTransaction(tx);
+            }
+        }
+        for (TransactionPlugin2 tp : transactionPlugins2) {
+            request = tp.beforeTransaction(request);
+        }
+
+        return request;
     }
 
-    void fireBeforeCommit(TransactionRequest request, TransactionPlugin tp) {
+    private void fireAfterTransaction(TransactionRequest request, TransactionResponse result, boolean committed) {
         TransactionType tx = TransactionRequest.WFS11.unadapt(request);
-        if (tx != null) tp.beforeCommit(tx);
+        TransactionResponseType tr = TransactionResponse.WFS11.unadapt(result);
+        if (tx != null && tr != null) {
+            for (TransactionPlugin tp : transactionPlugins) {
+                tp.afterTransaction(tx, tr, committed);
+            }
+        }
+        for (TransactionPlugin2 tp : transactionPlugins2) {
+            tp.afterTransaction(request, result, committed);
+        }
     }
 
-    void fireBeforeTransaction(TransactionRequest request, TransactionPlugin tp) {
-        TransactionType tx = TransactionRequest.WFS11.unadapt(request);
-        if (tx != null) tp.beforeTransaction(tx);
+    private void fireBeforeCommit(TransactionRequest request) {
+        // inform plugins we're about to commit
+        for (TransactionPlugin tp : transactionPlugins) {
+            TransactionType tx = TransactionRequest.WFS11.unadapt(request);
+            if (tx != null) {
+                tp.beforeCommit(tx);
+            }
+        }
+        for (TransactionPlugin2 tp : transactionPlugins2) {
+            tp.beforeCommit(request);
+        }
     }
 
+
     /**
      * Looks up the element handlers to be used for each element
      *
@@ -664,6 +684,7 @@ public void dataStoreChange(List listeners, TransactionEvent event)
         public void dataStoreChange(TransactionEvent event)
             throws WFSException {
             dataStoreChange(transactionPlugins, event);
+            dataStoreChange(transactionPlugins2, event);
             dataStoreChange(transactionListeners, event);
         }
     }

src/wfs/src/main/java/org/geoserver/wfs/TransactionPlugin.java

@@ -12,7 +12,9 @@
 /**
  * A transaction plugin is able to listen to a transaction evolution, perform
  * checks and throw exceptions, alter transaction requests, as well as
+ * @deprecated Use {@link TransactionPlugin2 instead}
  */
+@Deprecated
 public interface TransactionPlugin extends TransactionListener {
     /**
      * Check/alter the transaction request elements

src/wfs/src/main/java/org/geoserver/wfs/TransactionPlugin2.java

@@ -0,0 +1,47 @@
+/* (c) 2017 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.wfs;
+
+import org.geoserver.platform.ExtensionPriority;
+import org.geoserver.wfs.request.TransactionRequest;
+import org.geoserver.wfs.request.TransactionResponse;
+
+
+/**
+ * A transaction plugin is able to listen to a transaction evolution, perform
+ * checks and throw exceptions, alter transaction requests, as well as
+ */
+public interface TransactionPlugin2 extends TransactionListener, ExtensionPriority {
+    /**
+     * Check/alter the transaction request elements
+     */
+    TransactionRequest beforeTransaction(TransactionRequest request)
+        throws WFSException;
+
+    /**
+     * Say the last word before we actually commit the transaction
+     */
+    void beforeCommit(TransactionRequest request) throws WFSException;
+
+    /**
+     * Notification the transaction ended
+     * 
+     * @param request
+     *            the originating transaction request
+     * @param result
+     *            {@code null} if {@code committed == false}, the transaction result object to be
+     *            sent back to the client otherwise.
+     * 
+     * @param committed
+     *            true if the transaction was successful, false if the transaction was aborted for
+     *            any reason
+     */
+    void afterTransaction(TransactionRequest request, TransactionResponse result, boolean committed);
+
+    @Override
+    default int getPriority() {
+        return ExtensionPriority.LOWEST;
+    }
+}