[geode] branch develop updated: GEODE-7162: Remove AttributesFactory in geode-lucene (#4280)

[jj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jjramos pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 8525f2c  GEODE-7162: Remove AttributesFactory in geode-lucene (#4280)
8525f2c is described below

commit 8525f2c58ff3276a3a7f76616239d52994bb16f2
Author: Juan José Ramos <ju...@users.noreply.github.com>
AuthorDate: Fri Nov 8 12:11:36 2019 +0000

    GEODE-7162: Remove AttributesFactory in geode-lucene (#4280)
    
    - Fixed method setPartitionAttributes to behave as the one within
      the deprecated AttributesFactory class.
    - Removed references to the deprecated AttributesFactory from the
      geode-lucene module.
---
 .../cache/xmlcache/RegionAttributesCreation.java   | 29 +++++++++++++++++++---
 .../cache/lucene/test/LuceneTestUtilities.java     |  9 ++++---
 .../internal/LuceneIndexForPartitionedRegion.java  | 11 ++++----
 .../lucene/internal/LuceneRegionListener.java      |  9 ++++---
 .../LuceneIndexForPartitionedRegionTest.java       | 15 +++++------
 5 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/RegionAttributesCreation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/RegionAttributesCreation.java
index b435f13..1c46648 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/RegionAttributesCreation.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/RegionAttributesCreation.java
@@ -46,6 +46,7 @@ import org.apache.geode.cache.client.internal.InternalClientCache;
 import org.apache.geode.compression.Compressor;
 import org.apache.geode.internal.cache.EvictionAttributesImpl;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.cache.PartitionAttributesImpl;
 import org.apache.geode.internal.cache.PartitionedRegionHelper;
 import org.apache.geode.internal.cache.UserSpecifiedRegionAttributes;
 
@@ -1550,15 +1551,35 @@ public class RegionAttributesCreation extends UserSpecifiedRegionAttributes
     return this.partitionAttributes;
   }
 
-  public void setPartitionAttributes(PartitionAttributes pa) {
-    if (pa != null) {
+  public void setPartitionAttributes(PartitionAttributes partitionAttr) {
+    if (partitionAttr != null) {
       if (!hasDataPolicy()) {
-        this.setDataPolicy(PartitionedRegionHelper.DEFAULT_DATA_POLICY);
+        setDataPolicy(PartitionedRegionHelper.DEFAULT_DATA_POLICY);
         setHasDataPolicy(false);
+      } else if (!PartitionedRegionHelper.ALLOWED_DATA_POLICIES.contains(getDataPolicy())) {
+        throw new IllegalStateException(
+            String.format(
+                "Data policy %s is not allowed for a partitioned region. DataPolicies other than %s are not allowed.",
+                this.getDataPolicy(), PartitionedRegionHelper.ALLOWED_DATA_POLICIES));
+      }
+
+      if (hasPartitionAttributes()
+          && partitionAttributes instanceof PartitionAttributesImpl
+          && partitionAttr instanceof PartitionAttributesImpl) {
+
+        // Make a copy and call merge on it to prevent bug 51616
+        PartitionAttributesImpl copy = ((PartitionAttributesImpl) partitionAttributes).copy();
+        copy.merge((PartitionAttributesImpl) partitionAttr);
+        this.partitionAttributes = copy;
+      } else {
+        this.partitionAttributes = partitionAttr;
       }
 
-      this.partitionAttributes = pa;
       setHasPartitionAttributes(true);
+      ((PartitionAttributesImpl) partitionAttributes).setOffHeap(offHeap);
+    } else {
+      partitionAttributes = null;
+      setHasPartitionAttributes(false);
     }
   }
 
diff --git a/geode-lucene/geode-lucene-test/src/main/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java b/geode-lucene/geode-lucene-test/src/main/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java
index 684bed1..1f402a5 100644
--- a/geode-lucene/geode-lucene-test/src/main/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java
+++ b/geode-lucene/geode-lucene-test/src/main/java/org/apache/geode/cache/lucene/test/LuceneTestUtilities.java
@@ -34,13 +34,14 @@ import org.apache.lucene.document.FloatPoint;
 import org.apache.lucene.document.IntPoint;
 import org.apache.lucene.search.Query;
 
-import org.apache.geode.cache.AttributesFactory;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.EntryOperation;
 import org.apache.geode.cache.FixedPartitionAttributes;
 import org.apache.geode.cache.FixedPartitionResolver;
 import org.apache.geode.cache.PartitionAttributesFactory;
 import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.cache.asyncqueue.AsyncEventQueue;
 import org.apache.geode.cache.asyncqueue.internal.AsyncEventQueueImpl;
 import org.apache.geode.cache.lucene.LuceneIndex;
@@ -162,7 +163,7 @@ public class LuceneTestUtilities {
       allPartitions.add("Q2");
     }
 
-    AttributesFactory fact = new AttributesFactory();
+    RegionFactory regionFactory = cache.createRegionFactory(RegionShortcut.PARTITION);
 
     PartitionAttributesFactory pfact = new PartitionAttributesFactory();
     pfact.setTotalNumBuckets(16);
@@ -174,8 +175,8 @@ public class LuceneTestUtilities {
       }
     }
     pfact.setPartitionResolver(new MyFixedPartitionResolver(allPartitions));
-    fact.setPartitionAttributes(pfact.create());
-    Region r = cache.createRegionFactory(fact.create()).create(regionName);
+    regionFactory.setPartitionAttributes(pfact.create());
+    Region r = regionFactory.create(regionName);
     assertNotNull(r);
     return r;
   }
diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
index 17de988..8af5338 100644
--- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
@@ -19,7 +19,6 @@ import java.util.Set;
 import java.util.concurrent.ExecutorService;
 
 import org.apache.geode.CancelException;
-import org.apache.geode.cache.AttributesFactory;
 import org.apache.geode.cache.FixedPartitionResolver;
 import org.apache.geode.cache.PartitionAttributes;
 import org.apache.geode.cache.PartitionAttributesFactory;
@@ -45,6 +44,7 @@ import org.apache.geode.distributed.internal.membership.InternalDistributedMembe
 import org.apache.geode.internal.cache.BucketRegion;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.PartitionedRegion;
+import org.apache.geode.internal.cache.xmlcache.RegionAttributesCreation;
 
 public class LuceneIndexForPartitionedRegion extends LuceneIndexImpl {
   protected Region fileAndChunkRegion;
@@ -173,14 +173,13 @@ public class LuceneIndexForPartitionedRegion extends LuceneIndexImpl {
     partitionAttributesFactory.setColocatedWith(colocatedWithRegionName);
     configureLuceneRegionAttributesFactory(partitionAttributesFactory, partitionAttributes);
 
-    // Create AttributesFactory based on input RegionShortcut
+    // Create RegionAttributes based on input RegionShortcut
     RegionAttributes baseAttributes = this.cache.getRegionAttributes(regionShortCut.toString());
-    AttributesFactory factory = new AttributesFactory(baseAttributes);
-    factory.setPartitionAttributes(partitionAttributesFactory.create());
+    RegionAttributesCreation attributes = new RegionAttributesCreation(baseAttributes, false);
+    attributes.setPartitionAttributes(partitionAttributesFactory.create());
     if (regionAttributes.getDataPolicy().withPersistence()) {
-      factory.setDiskStoreName(regionAttributes.getDiskStoreName());
+      attributes.setDiskStoreName(regionAttributes.getDiskStoreName());
     }
-    RegionAttributes<K, V> attributes = factory.create();
 
     return createRegion(regionName, attributes);
   }
diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneRegionListener.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneRegionListener.java
index 9439b76..a84250e 100644
--- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneRegionListener.java
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneRegionListener.java
@@ -20,7 +20,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.analysis.Analyzer;
 
-import org.apache.geode.cache.AttributesFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionAttributes;
 import org.apache.geode.cache.lucene.LuceneIndexDestroyedException;
@@ -29,6 +28,7 @@ import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.InternalRegionArguments;
 import org.apache.geode.internal.cache.PartitionedRegion;
 import org.apache.geode.internal.cache.RegionListener;
+import org.apache.geode.internal.cache.xmlcache.RegionAttributesCreation;
 import org.apache.geode.logging.internal.log4j.api.LogService;
 
 public class LuceneRegionListener implements RegionListener {
@@ -90,9 +90,10 @@ public class LuceneRegionListener implements RegionListener {
 
       String aeqId = LuceneServiceImpl.getUniqueIndexName(this.indexName, this.regionPath);
       if (!attrs.getAsyncEventQueueIds().contains(aeqId)) {
-        AttributesFactory af = new AttributesFactory(attrs);
-        af.addAsyncEventQueueId(aeqId);
-        updatedRA = af.create();
+        RegionAttributesCreation regionAttributesCreation =
+            new RegionAttributesCreation(attrs, false);
+        regionAttributesCreation.addAsyncEventQueueId(aeqId);
+        updatedRA = regionAttributesCreation;
       }
 
       // Add index creation profile
diff --git a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java
index 3435f4d..7ad8406 100644
--- a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java
+++ b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegionTest.java
@@ -35,7 +35,6 @@ import org.junit.experimental.categories.Category;
 import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 
-import org.apache.geode.cache.AttributesFactory;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheListener;
 import org.apache.geode.cache.DataPolicy;
@@ -56,6 +55,7 @@ import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.PartitionedRegion;
 import org.apache.geode.internal.cache.extension.ExtensionPoint;
+import org.apache.geode.internal.cache.xmlcache.RegionAttributesCreation;
 import org.apache.geode.test.fake.Fakes;
 import org.apache.geode.test.junit.categories.LuceneTest;
 
@@ -222,15 +222,15 @@ public class LuceneIndexForPartitionedRegionTest {
 
   private RegionAttributes createRegionAttributes(final boolean withPersistence,
       PartitionAttributes partitionAttributes) {
-    AttributesFactory factory = new AttributesFactory();
+    RegionAttributesCreation regionAttributes = new RegionAttributesCreation();
     if (withPersistence) {
-      factory.setDataPolicy(DataPolicy.PERSISTENT_PARTITION);
+      regionAttributes.setDataPolicy(DataPolicy.PERSISTENT_PARTITION);
     } else {
-      factory.setDataPolicy(DataPolicy.PARTITION);
+      regionAttributes.setDataPolicy(DataPolicy.PARTITION);
     }
-    factory.setPartitionAttributes(partitionAttributes);
-    RegionAttributes ra = factory.create();
-    return ra;
+
+    regionAttributes.setPartitionAttributes(partitionAttributes);
+    return regionAttributes;
   }
 
   private Region initializeScenario(final boolean withPersistence, final String regionPath,
@@ -254,6 +254,7 @@ public class LuceneIndexForPartitionedRegionTest {
   private PartitionAttributes initializeAttributes(final Cache cache) {
     PartitionAttributes partitionAttributes = mock(PartitionAttributes.class);
     RegionAttributes attributes = mock(RegionAttributes.class);
+    when(attributes.getDataPolicy()).thenReturn(DataPolicy.PARTITION);
     when(attributes.getCacheListeners()).thenReturn(new CacheListener[0]);
     when(attributes.getRegionTimeToLive()).thenReturn(ExpirationAttributes.DEFAULT);
     when(attributes.getRegionIdleTimeout()).thenReturn(ExpirationAttributes.DEFAULT);