This is an automated email from the ASF dual-hosted git repository.
gosullivan 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 ff66270 GEODE-5208 remove test hooks from TypeRegistration (#1958)
ff66270 is described below
commit ff662703a48991ed45b2f42ce1ee699142016e4f
Author: Galen O'Sullivan <gosulli...@pivotal.io>
AuthorDate: Wed May 23 11:22:53 2018 -0700
GEODE-5208 remove test hooks from TypeRegistration (#1958)
GEODE-5208: remove test hooks from TypeRegistration
* Add `@Override` to some methods that didn't have it.
* delete some comments and make some methods less public.
* Remove getClassToType() by checking the same thing another way.
---
.../geode/pdx/internal/ClientTypeRegistration.java | 3 --
.../geode/pdx/internal/LonerTypeRegistration.java | 17 ++++----
.../geode/pdx/internal/NullTypeRegistration.java | 20 +++++----
.../geode/pdx/internal/PeerTypeRegistration.java | 46 +------------------
.../geode/pdx/internal/TypeRegistration.java | 10 -----
.../apache/geode/pdx/internal/TypeRegistry.java | 24 +---------
.../geode/cache/query/dunit/PdxQueryDUnitTest.java | 51 ++++++----------------
.../membership/gms/MembershipManagerHelper.java | 33 ++------------
.../apache/geode/pdx/PdxSerializableJUnitTest.java | 4 --
9 files changed, 38 insertions(+), 170 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/pdx/internal/ClientTypeRegistration.java
b/geode-core/src/main/java/org/apache/geode/pdx/internal/ClientTypeRegistration.java
index 408cb46..1019291 100644
---
a/geode-core/src/main/java/org/apache/geode/pdx/internal/ClientTypeRegistration.java
+++
b/geode-core/src/main/java/org/apache/geode/pdx/internal/ClientTypeRegistration.java
@@ -266,9 +266,6 @@ public class ClientTypeRegistration implements
TypeRegistration {
}
@Override
- public void testClearRegistry() {}
-
- @Override
public boolean isClient() {
return true;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/pdx/internal/LonerTypeRegistration.java
b/geode-core/src/main/java/org/apache/geode/pdx/internal/LonerTypeRegistration.java
index 9d78a57..3215fd0 100644
---
a/geode-core/src/main/java/org/apache/geode/pdx/internal/LonerTypeRegistration.java
+++
b/geode-core/src/main/java/org/apache/geode/pdx/internal/LonerTypeRegistration.java
@@ -34,35 +34,36 @@ public class LonerTypeRegistration implements
TypeRegistration {
this.cache = cache;
}
+ @Override
public int defineType(PdxType newType) {
initializeRegistry();
return delegate.defineType(newType);
}
+ @Override
public PdxType getType(int typeId) {
initializeRegistry();
return delegate.getType(typeId);
}
+ @Override
public void addRemoteType(int typeId, PdxType type) {
initializeRegistry();
delegate.addRemoteType(typeId, type);
}
- public int getLastAllocatedTypeId() {
- initializeRegistry();
- return delegate.getLastAllocatedTypeId();
- }
-
+ @Override
public void initialize() {
// do nothing. This type registry is initialized lazily.
}
+ @Override
public void gatewaySenderStarted(GatewaySender gatewaySender) {
initializeRegistry(false);
delegate.gatewaySenderStarted(gatewaySender);
}
+ @Override
public void creatingPersistentRegion() {
if (delegate != null) {
delegate.creatingPersistentRegion();
@@ -70,6 +71,7 @@ public class LonerTypeRegistration implements
TypeRegistration {
}
+ @Override
public void creatingPool() {
initializeRegistry(true);
delegate.creatingPool();
@@ -104,7 +106,7 @@ public class LonerTypeRegistration implements
TypeRegistration {
*
* @return true if this member is a loner and we can't determine what type
of registry they want.
*/
- public static boolean isIndeterminateLoner(InternalCache cache) {
+ static boolean isIndeterminateLoner(InternalCache cache) {
boolean isLoner = cache.getInternalDistributedSystem().isLoner();
boolean pdxConfigured = cache.getPdxPersistent();
return isLoner && !pdxConfigured/* && !hasGateways */;
@@ -153,9 +155,6 @@ public class LonerTypeRegistration implements
TypeRegistration {
}
@Override
- public void testClearRegistry() {}
-
- @Override
public boolean isClient() {
return delegate.isClient();
}
diff --git
a/geode-core/src/main/java/org/apache/geode/pdx/internal/NullTypeRegistration.java
b/geode-core/src/main/java/org/apache/geode/pdx/internal/NullTypeRegistration.java
index 625383e..91ccb6c 100644
---
a/geode-core/src/main/java/org/apache/geode/pdx/internal/NullTypeRegistration.java
+++
b/geode-core/src/main/java/org/apache/geode/pdx/internal/NullTypeRegistration.java
@@ -27,50 +27,57 @@ import org.apache.geode.pdx.PdxInitializationException;
*/
public class NullTypeRegistration implements TypeRegistration {
+ @Override
public int defineType(PdxType newType) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
+ @Override
public PdxType getType(int typeId) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
+ @Override
public void addRemoteType(int typeId, PdxType type) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
- public int getLastAllocatedTypeId() {
- throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
- }
-
+ @Override
public void initialize() {
// do nothing
}
+ @Override
public void gatewaySenderStarted(GatewaySender gatewaySender) {
// do nothing
}
+ @Override
public void creatingPersistentRegion() {
// do nothing
}
+ @Override
public void creatingPool() {
// do nothing
}
+ @Override
public int getEnumId(Enum<?> v) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
+ @Override
public void addRemoteEnum(int enumId, EnumInfo newInfo) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
+ @Override
public int defineEnum(EnumInfo newInfo) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
+ @Override
public EnumInfo getEnumById(int enumId) {
throw new PdxInitializationException("Trying to use PDX type, but type
registry is disabled");
}
@@ -96,11 +103,6 @@ public class NullTypeRegistration implements
TypeRegistration {
}
@Override
- public void testClearRegistry() {
-
- }
-
- @Override
public boolean isClient() {
return false;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
index 72dbb4c..eff754d 100644
---
a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
+++
b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
@@ -66,11 +66,7 @@ public class PeerTypeRegistration implements
TypeRegistration {
public static final String LOCK_SERVICE_NAME = "__PDX";
- /**
- * The region name. Public for tests only.
- */
public static final String REGION_NAME = "PdxTypes";
-
public static final String REGION_FULL_PATH = "/" + REGION_NAME;
public static final int PLACE_HOLDER_FOR_TYPE_ID = 0xFFFFFF;
public static final int PLACE_HOLDER_FOR_DS_ID = 0xFF000000;
@@ -342,29 +338,6 @@ public class PeerTypeRegistration implements
TypeRegistration {
}
}
- private int lastAllocatedTypeId; // for unit tests
- private int lastAllocatedEnumId; // for unit tests
-
- /**
- * Test hook that returns the most recently allocated type id
- *
- * @return the most recently allocated type id
- */
- public int getLastAllocatedTypeId() {
- verifyConfiguration();
- return this.lastAllocatedTypeId;
- }
-
- /**
- * Test hook that returns the most recently allocated enum id
- *
- * @return the most recently allocated enum id
- */
- public int getLastAllocatedEnumId() {
- verifyConfiguration();
- return this.lastAllocatedEnumId;
- }
-
public int defineType(PdxType newType) {
verifyConfiguration();
Integer existingId = typeToId.get(newType);
@@ -720,7 +693,7 @@ public class PeerTypeRegistration implements
TypeRegistration {
@Override
public Map<Integer, PdxType> types() {
// ugh, I don't think we can rely on the local map to contain all types
- Map<Integer, PdxType> types = new HashMap<Integer, PdxType>();
+ Map<Integer, PdxType> types = new HashMap<>();
for (Entry<Object, Object> type : getIdToType().entrySet()) {
Object id = type.getKey();
if (type.getValue() instanceof PdxType) {
@@ -783,23 +756,6 @@ public class PeerTypeRegistration implements
TypeRegistration {
}
}
- /**
- * For testing purpose
- */
- public Map<String, CopyOnWriteHashSet<PdxType>> getClassToType() {
- return classToType;
- }
-
- /**
- * test hook
- */
- @Override
- public void testClearRegistry() {
- idToType.clear();
- enumToId.clear();
- typeToId.clear();
- }
-
@Override
public boolean isClient() {
return false;
diff --git
a/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistration.java
b/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistration.java
index c263f9a..4857e27 100644
---
a/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistration.java
+++
b/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistration.java
@@ -42,11 +42,6 @@ public interface TypeRegistration {
void addImportedType(int typeId, PdxType importedType);
- /**
- * Test hook to get the last allocated type id
- */
- int getLastAllocatedTypeId();
-
void initialize();
void gatewaySenderStarted(GatewaySender gatewaySender);
@@ -92,11 +87,6 @@ public interface TypeRegistration {
*/
Set<PdxType> getPdxTypesForClassName(String className);
- /*
- * test hook
- */
- void testClearRegistry();
-
boolean isClient();
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistry.java
b/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistry.java
index 93fa670..afd3f07 100644
--- a/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistry.java
+++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/TypeRegistry.java
@@ -83,17 +83,6 @@ public class TypeRegistry {
}
}
- /*
- * Test Hook to clear the type registry
- */
- public void testClearTypeRegistry() {
- this.typeToId.clear();
- this.idToType.clear();
- this.idToEnum.clear();
- this.enumInfoToId.clear();
- this.distributedTypeRegistry.testClearRegistry();
- }
-
public void testClearLocalTypeRegistry() {
this.localTypeIds.clear();
this.localTypeIdMaps.clear();
@@ -252,17 +241,6 @@ public class TypeRegistry {
return newType;
}
- /**
- * Test hook that returns the most recently allocated type id
- *
- * Note that this method will not work on clients.
- *
- * @return the most recently allocated type id
- */
- public int getLastAllocatedTypeId() {
- return this.distributedTypeRegistry.getLastAllocatedTypeId();
- }
-
public TypeRegistration getTypeRegistration() {
return this.distributedTypeRegistry;
}
@@ -536,7 +514,7 @@ public class TypeRegistry {
/**
* Get the size of the the type registry in this local member
*/
- public int getLocalSize() {
+ int getLocalSize() {
int result = this.distributedTypeRegistry.getLocalSize();
if (result == 0) {
// If this is the client, go ahead and return the number of cached types
we have
diff --git
a/geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java
index 15b7874..c85fd0b 100644
---
a/geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java
@@ -16,6 +16,7 @@ package org.apache.geode.cache.query.dunit;
import static org.apache.geode.internal.Assert.fail;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import java.util.Map;
import java.util.Properties;
@@ -49,7 +50,6 @@ import org.apache.geode.cache.server.CacheServer;
import org.apache.geode.cache30.CacheSerializableRunnable;
import org.apache.geode.cache30.ClientServerTestCase;
import org.apache.geode.internal.AvailablePortHelper;
-import org.apache.geode.internal.CopyOnWriteHashSet;
import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.pdx.FieldType;
@@ -69,6 +69,7 @@ import org.apache.geode.test.dunit.Invoke;
import org.apache.geode.test.dunit.NetworkUtils;
import org.apache.geode.test.dunit.SerializableCallable;
import org.apache.geode.test.dunit.SerializableRunnable;
+import org.apache.geode.test.dunit.SerializableRunnableIF;
import org.apache.geode.test.dunit.VM;
import org.apache.geode.test.junit.categories.DistributedTest;
import org.apache.geode.test.junit.categories.OQLQueryTest;
@@ -2571,7 +2572,7 @@ public class PdxQueryDUnitTest extends PDXQueryTestBase {
final int port1 = (Integer) vm0.invoke(new SerializableCallable("Create
Server1") {
@Override
public Object call() throws Exception {
- Region r1 =
getCache().createRegionFactory(RegionShortcut.REPLICATE).create(regionName);
+
getCache().createRegionFactory(RegionShortcut.REPLICATE).create(regionName);
CacheServer server = getCache().addCacheServer();
int port = AvailablePortHelper.getRandomAvailablePortForDUnitSite();
@@ -2641,43 +2642,19 @@ public class PdxQueryDUnitTest extends PDXQueryTestBase
{
}
});
- // on server 1 verify local map in PeerTypeRegistration has fields
- vm0.invoke(new SerializableCallable("Create client") {
- @Override
- public Object call() throws Exception {
- TypeRegistration registration =
getCache().getPdxRegistry().getTypeRegistration();
- Assert.assertTrue(registration instanceof PeerTypeRegistration);
- Map<String, CopyOnWriteHashSet<PdxType>> m =
- ((PeerTypeRegistration) registration).getClassToType();
- assertEquals(1, m.size());
- assertEquals("PdxVersionedNewPortfolio", m.keySet().iterator().next());
- assertEquals(2, m.values().iterator().next().size());
- for (PdxType p : m.values().iterator().next()) {
- assertEquals("PdxVersionedNewPortfolio", p.getClassName());
- }
- return null;
- }
- });
+ final SerializableRunnableIF verifyTwoPdxTypesArePresent = () -> {
+ TypeRegistration registration =
getCache().getPdxRegistry().getTypeRegistration();
+ assertTrue(registration instanceof PeerTypeRegistration);
- // on server 2 verify local map in PeerTypeRegistration has fields
- vm1.invoke(new SerializableCallable("Create client") {
- @Override
- public Object call() throws Exception {
- TypeRegistration registration =
getCache().getPdxRegistry().getTypeRegistration();
- Assert.assertTrue(registration instanceof PeerTypeRegistration);
- Map<String, CopyOnWriteHashSet<PdxType>> m =
- ((PeerTypeRegistration) registration).getClassToType();
- assertEquals(1, m.size());
- assertEquals("PdxVersionedNewPortfolio", m.keySet().iterator().next());
- assertEquals(2, m.values().iterator().next().size());
- for (PdxType p : m.values().iterator().next()) {
- assertEquals("PdxVersionedNewPortfolio", p.getClassName());
- }
- return null;
+ final Map<Integer, PdxType> types = registration.types();
+ assertEquals(2, types.size());
+ for (PdxType type : types.values()) {
+ assertEquals("PdxVersionedNewPortfolio", type.getClassName());
}
- });
+ };
- Invoke.invokeInEveryVM(DistributedTestCase.class, "disconnectFromDS");
+ vm0.invoke(verifyTwoPdxTypesArePresent);
+ vm1.invoke(verifyTwoPdxTypesArePresent);
}
/**
@@ -3188,7 +3165,7 @@ public class PdxQueryDUnitTest extends PDXQueryTestBase {
}
// check if the types registered on server are fetched by the client
TypeRegistration registration =
getCache().getPdxRegistry().getTypeRegistration();
- Assert.assertTrue(registration instanceof ClientTypeRegistration);
+ assertTrue(registration instanceof ClientTypeRegistration);
Map<Integer, PdxType> m = ((ClientTypeRegistration)
registration).types();
assertEquals(2, m.size());
for (PdxType type : m.values()) {
diff --git
a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/MembershipManagerHelper.java
b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/MembershipManagerHelper.java
index 2b5352c..645ae0b 100644
---
a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/MembershipManagerHelper.java
+++
b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/MembershipManagerHelper.java
@@ -19,7 +19,6 @@ import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.DistributedSystem;
import org.apache.geode.distributed.internal.ClusterDistributionManager;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.distributed.internal.membership.MembershipManager;
import org.apache.geode.distributed.internal.membership.gms.interfaces.Manager;
import
org.apache.geode.distributed.internal.membership.gms.mgr.GMSMembershipManager;
@@ -37,8 +36,7 @@ public class MembershipManagerHelper {
public static MembershipManager getMembershipManager(DistributedSystem sys) {
InternalDistributedSystem isys = (InternalDistributedSystem) sys;
ClusterDistributionManager dm = (ClusterDistributionManager) isys.getDM();
- MembershipManager mgr = dm.getMembershipManager();
- return mgr;
+ return dm.getMembershipManager();
}
/**
@@ -64,10 +62,6 @@ public class MembershipManagerHelper {
}
}
- public static void beHealthyMember(DistributedSystem sys) {
- ((Manager) getMembershipManager(sys)).beHealthy();
- }
-
/** returns the current coordinator address */
public static DistributedMember getCoordinator(DistributedSystem sys) {
return ((Manager) getMembershipManager(sys)).getCoordinator();
@@ -90,25 +84,6 @@ public class MembershipManagerHelper {
getMembershipManager(sys).unregisterTestHook(hook);
}
- // /**
- // * returns the view lock. Holding this lock will prevent the processing
- // * of new views, and will prevent other threads from being able to access
- // * the view
- // */
- // public static Object getViewLock(DistributedSystem sys) {
- // return getMembershipManager(sys).latestViewLock;
- // }
-
- /** returns true if the given member is shunned */
- public static boolean isShunned(DistributedSystem sys, DistributedMember
mbr) {
- return ((Manager) getMembershipManager(sys)).isShunned(mbr);
- }
-
- /** returns true if the given member is a surprise member */
- public static boolean isSurpriseMember(DistributedSystem sys,
DistributedMember mbr) {
- return getMembershipManager(sys).isSurpriseMember(mbr);
- }
-
/**
* add a member id to the surprise members set, with the given millisecond
clock birth time
*/
@@ -133,13 +108,11 @@ public class MembershipManagerHelper {
final DistributedMember member, final long timeout) {
WaitCriterion ev = new WaitCriterion() {
public boolean done() {
- return
!getMembershipManager(sys).getView().contains((InternalDistributedMember)
member);
+ return !getMembershipManager(sys).getView().contains(member);
}
public String description() {
- String assMsg =
- "Waited over " + timeout + " ms for " + member + " to depart, but
it didn't";
- return assMsg;
+ return "Waited over " + timeout + " ms for " + member + " to depart,
but it didn't";
}
};
Wait.waitForCriterion(ev, timeout, 200, true);
diff --git
a/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableJUnitTest.java
index 04b44fd..13ef7c7 100644
---
a/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/pdx/PdxSerializableJUnitTest.java
@@ -87,10 +87,6 @@ public class PdxSerializableJUnitTest {
this.cache.close();
}
- private int getLastPdxTypeId() {
- return this.cache.getPdxRegistry().getLastAllocatedTypeId();
- }
-
private int getPdxTypeIdForClass(Class c) {
// here we are assuming Dsid == 0
return this.cache.getPdxRegistry().getExistingTypeForClass(c).hashCode()
--
To stop receiving notification emails like this one, please contact
gosulli...@apache.org.
