From 332338d018dbb7a491a669e0b749ea20fd157b7a Mon Sep 17 00:00:00 2001
From: Clebert Suconic <clebertsuconic@apache.org>
Date: Wed, 14 Dec 2016 12:01:16 -0500
Subject: [PATCH] ARTEMIS-890 Improving Paging consistencies with
 broker.persistent = false. Block, Page and Drop will now work under non
 persistent

---
 .../activemq/artemis/cli/commands/Create.java     |  6 ++++++
 .../activemq/artemis/cli/commands/etc/broker.xml  |  2 +-
 .../artemis/core/paging/impl/PagingStoreImpl.java |  8 ++++++--
 .../impl/nullpm/NullStorageManager.java           | 15 +++++++++++++--
 .../tests/integration/paging/PagingTest.java      | 13 +++++++++++--
 5 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java
index 7175c8dd1b..5faa7a7338 100644
--- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java
+++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java
@@ -714,6 +714,12 @@ public class Create extends InputAbstract {
          filters.put("${hornetq-acceptor}", applyFilters(readTextFile(ETC_HORNETQ_ACCEPTOR_TXT), filters));
       }
 
+      if (disablePersistence) {
+         filters.put("${full-policy}", "BLOCK");
+      } else {
+         filters.put("${full-policy}", "PAGE");
+      }
+
       performAutoTune(filters, aio, dataFolder);
 
       write(ETC_BROKER_XML, filters, false);
diff --git a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml
index ea51eb07fd..3e0d4e1614 100644
--- a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml
+++ b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml
@@ -93,7 +93,7 @@ ${cluster-security.settings}${cluster.settings}${replicated.settings}${shared-st
             <!-- with -1 only the global-max-size is in use for limiting -->
             <max-size-bytes>-1</max-size-bytes>
             <message-counter-history-day-limit>10</message-counter-history-day-limit>
-            <address-full-policy>PAGE</address-full-policy>
+            <address-full-policy>${full-policy}</address-full-policy>
          </address-setting>
       </address-settings>
 
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java
index ce21081ba7..54992e464f 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java
@@ -797,7 +797,9 @@ public class PagingStoreImpl implements PagingStore {
          lock.readLock().unlock();
       }
 
-      managerLock.lock();
+      if (managerLock != null) {
+         managerLock.lock();
+      }
       try {
          lock.writeLock().lock();
 
@@ -852,7 +854,9 @@ public class PagingStoreImpl implements PagingStore {
             lock.writeLock().unlock();
          }
       } finally {
-         managerLock.unlock();
+         if (managerLock != null) {
+            managerLock.unlock();
+         }
       }
    }
 
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/nullpm/NullStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/nullpm/NullStorageManager.java
index fac108e40e..21548798e3 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/nullpm/NullStorageManager.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/nullpm/NullStorageManager.java
@@ -540,12 +540,23 @@ public class NullStorageManager implements StorageManager {
       // no-op
    }
 
+
    @Override
-   public boolean addToPage(PagingStore s,
+   public boolean addToPage(PagingStore store,
                             ServerMessage msg,
                             Transaction tx,
                             RouteContextList listCtx) throws Exception {
-      return false;
+      /**
+       * Exposing the read-lock here is an encapsulation violation done in order to keep the code
+       * simpler. The alternative would be to add a second method, say 'verifyPaging', to
+       * PagingStore.
+       * <p>
+       * Adding this second method would also be more surprise prone as it would require a certain
+       * calling order.
+       * <p>
+       * The reasoning is that exposing the lock is more explicit and therefore `less bad`.
+       */
+      return store.page(msg, tx, listCtx, null);
    }
 
    @Override
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java
index 21dd5b6d5d..d6ac105fef 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java
@@ -3329,7 +3329,16 @@ public class PagingTest extends ActiveMQTestBase {
    }
 
    @Test
-   public void testDropMessages() throws Exception {
+   public void testDropMessagesPersistent() throws Exception {
+      testDropMessages(true);
+   }
+
+   @Test
+   public void testDropMessagesNonPersistent() throws Exception {
+      testDropMessages(false);
+   }
+
+   public void testDropMessages(final boolean persistent) throws Exception {
       clearDataRecreateServerDirs();
 
       HashMap<String, AddressSettings> settings = new HashMap<>();
@@ -3339,7 +3348,7 @@ public class PagingTest extends ActiveMQTestBase {
 
       settings.put(PagingTest.ADDRESS.toString(), set);
 
-      server = createServer(true, createDefaultInVMConfig(), 1024, 10 * 1024, settings);
+      server = createServer(persistent, createDefaultInVMConfig(), 1024, 10 * 1024, settings);
 
       server.start();