GEODE-7704: Replacing inheritance of MemberIdentifierImpl with delaga…

…tion (apache#4636)

* Deduplicate fields betweeen MemberIdentifierImpl and GMSMemberData

Moving all state down into GMSMemberData and removing duplicate fields
isPartial and versionObject. GMSMemberData now has all of the state of a
member.

* Delegate rather than inherit InternalDistributedMember

Delegating to MemberIdentifierImpl, rather than inheriting from it.

* Delegate to MemberIdentifier, rather than MemberIdentifierImpl

Having InternalDistributedMember delegate to MemberIdentifier, rather than
MemberIdentifierImpl, so that we are not relying on the concrete class and all
required methods are part of the interface. As a result, adding a number of
additional methods to the MemberIdentifier interface.

* Removing uses of getMemberData

Removing uses of getMemberData and adding methods to MemberIdentifier that
delegate to memberData.

* Hiding MemberIdentiferImpl

Using MemberIdentifier everywhere insteand of MemberIdentiferImpl. Still need to
move MemberIdentiferImpl to an internal package.

* Making PMD happy

Marking the MemberIdentifierFactory as immutable.

* Fixing NPE in getGroups

The MemberIdentifierImpl.getGroups needs to check for a null array. Places that
used to call getMemberData.getGroups were throwing an NPE.

* Fixing a test that used MemberIdentifierFactoryImpl

This factory is not going to play well with other parts of the test that are
generating InternalDistributedMembers.

* Fixing AnalyzeSerializablesJUnitTest

InternalDistributedMember now has new serialization methods that delegate to
the methods that it previously was just inheriting.

* Fixing a unit test failure due to mocking MemberData

InternalDistributedMemberTest was mocking the underlying MemberData and
changing state on the InternalDistributedMember. Now that all state is
delegated to the MemberData, this test was failing because the mock was not
reflecting the state changes.

Co-authored-by: Ernie Burghardt <[email protected]>

geode-core/src/distributedTest/java/org/apache/geode/cache30/DistributedAckRegionCCEDUnitTest.java

@@ -40,7 +40,6 @@
 import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.DistributionAdvisor;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
-import org.apache.geode.distributed.internal.membership.api.MemberData;
 import org.apache.geode.internal.cache.DistributedCacheOperation;
 import org.apache.geode.internal.cache.DistributedRegion;
 import org.apache.geode.internal.cache.EntryEventImpl;
@@ -218,8 +217,8 @@ public void vMotionDuringGII(Set recipientSet, LocalRegion region) {
       VersionTag<InternalDistributedMember> tag =
           (VersionTag<InternalDistributedMember>) versionStamp.asVersionTag();
       // create a fake member ID that will be < mine and lose a concurrency check
-      MemberData nm =
-          CCRegion.getDistributionManager().getDistributionManagerId().getMemberData();
+      InternalDistributedMember nm =
+          CCRegion.getDistributionManager().getDistributionManagerId();
       InternalDistributedMember mbr = null;
 
       mbr = new InternalDistributedMember(nm.getInetAddress().getCanonicalHostName(),

geode-core/src/distributedTest/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java

@@ -263,7 +263,7 @@ private InternalDistributedMember connectAndSetUpPartialID() throws Exception {
     assertTrue(system == basicGetSystem()); // senders will use basicGetSystem()
     InternalDistributedMember internalDistributedMember = system.getDistributedMember();
 
-    internalDistributedMember.getMemberData().setName(null);
+    internalDistributedMember.setName(null);
     HeapDataOutputStream outputStream = new HeapDataOutputStream(100);
     internalDistributedMember.writeEssentialData(outputStream);
     DataInputStream dataInputStream =

geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java

@@ -362,10 +362,10 @@ public void testOldClientIDDeserialization() throws Exception {
     assertThat(newMemberID.getVersionObject()).isEqualTo(Version.GFE_82);
     assertThat(newID.getClientVersion()).isEqualTo(Version.GFE_82);
 
-    assertThat(newMemberID.getMemberData().getUuidLeastSignificantBits()).isEqualTo(0);
-    assertThat(newMemberID.getMemberData().getUuidMostSignificantBits()).isEqualTo(0);
+    assertThat(newMemberID.getUuidLeastSignificantBits()).isEqualTo(0);
+    assertThat(newMemberID.getUuidMostSignificantBits()).isEqualTo(0);
 
-    gmsID.getMemberData().setUUID(new UUID(1234L, 5678L));
+    gmsID.setUUID(new UUID(1234L, 5678L));
     memberID.setVersionObjectForTest(Version.CURRENT);
     clientID = ClientProxyMembershipID.getClientId(memberID);
     out = new HeapDataOutputStream(Version.CURRENT);
@@ -378,10 +378,10 @@ public void testOldClientIDDeserialization() throws Exception {
     assertThat(newMemberID.getVersionObject()).isEqualTo(Version.CURRENT);
     assertThat(newID.getClientVersion()).isEqualTo(Version.CURRENT);
 
-    assertThat(newMemberID.getMemberData().getUuidLeastSignificantBits())
-        .isEqualTo(gmsID.getMemberData().getUuidLeastSignificantBits());
-    assertThat(newMemberID.getMemberData().getUuidMostSignificantBits())
-        .isEqualTo(gmsID.getMemberData().getUuidMostSignificantBits());
+    assertThat(newMemberID.getUuidLeastSignificantBits())
+        .isEqualTo(gmsID.getUuidLeastSignificantBits());
+    assertThat(newMemberID.getUuidMostSignificantBits())
+        .isEqualTo(gmsID.getUuidMostSignificantBits());
   }
 
   @Test

geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/MembershipOnlyTest.java

@@ -32,7 +32,6 @@
 import org.apache.geode.distributed.internal.membership.api.LifecycleListener;
 import org.apache.geode.distributed.internal.membership.api.MemberIdentifier;
 import org.apache.geode.distributed.internal.membership.api.MemberIdentifierFactoryImpl;
-import org.apache.geode.distributed.internal.membership.api.MemberIdentifierImpl;
 import org.apache.geode.distributed.internal.membership.api.MemberStartupException;
 import org.apache.geode.distributed.internal.membership.api.Membership;
 import org.apache.geode.distributed.internal.membership.api.MembershipBuilder;
@@ -91,19 +90,19 @@ public void locatorStarts() {
 
   @Test
   public void memberCanConnectToSelfHostedLocator() throws MemberStartupException {
-    Membership<MemberIdentifierImpl> membership = startMember("member", membershipLocator);
+    Membership<MemberIdentifier> membership = startMember("member", membershipLocator);
     assertThat(membership.getView().getMembers()).hasSize(1);
   }
 
   @Test
   public void twoMembersCanConnect() throws MemberStartupException {
-    Membership<MemberIdentifierImpl> member1 = startMember("member1", membershipLocator);
-    Membership<MemberIdentifierImpl> member2 = startMember("member2", null);
+    Membership<MemberIdentifier> member1 = startMember("member1", membershipLocator);
+    Membership<MemberIdentifier> member2 = startMember("member2", null);
     await().untilAsserted(() -> assertThat(member1.getView().getMembers()).hasSize(2));
     await().untilAsserted(() -> assertThat(member2.getView().getMembers()).hasSize(2));
   }
 
-  private Membership<MemberIdentifierImpl> startMember(String name,
+  private Membership<MemberIdentifier> startMember(String name,
       final MembershipLocator embeddedLocator)
       throws MemberStartupException {
     MembershipConfig config = new MembershipConfig() {
@@ -131,10 +130,11 @@ public String getName() {
     TcpClient locatorClient = new TcpClient(socketCreator, dsfidSerializer.getObjectSerializer(),
         dsfidSerializer.getObjectDeserializer());
 
-    LifecycleListener<MemberIdentifierImpl> lifeCycleListener = mock(LifecycleListener.class);
+    LifecycleListener<MemberIdentifier> lifeCycleListener = mock(LifecycleListener.class);
 
-    final Membership<MemberIdentifierImpl> membership =
-        MembershipBuilder.<MemberIdentifierImpl>newMembershipBuilder(
+
+    final Membership<MemberIdentifier> membership =
+        MembershipBuilder.<MemberIdentifier>newMembershipBuilder(
             socketCreator,
             locatorClient,
             dsfidSerializer,

geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java

@@ -852,9 +852,9 @@ public void executeTestClientSocketHandler(MemberIdentifier gmsMember,
 
     MemberIdentifier testMember =
         createGMSMember(Version.CURRENT_ORDINAL, viewId,
-            gmsMember.getMemberData().getUuidMostSignificantBits(),
-            gmsMember.getMemberData().getUuidLeastSignificantBits());
-    testMember.getMemberData().setUdpPort(9000);
+            gmsMember.getUuidMostSignificantBits(),
+            gmsMember.getUuidLeastSignificantBits());
+    testMember.setUdpPort(9000);
 
     // We set to our expected test viewId in the IDM as well as resetting the gms member
     gmsMember.setVmViewId(viewId);

geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java

@@ -632,7 +632,7 @@ public void testRemoveRequestCausesForcedDisconnectInRogue() throws Exception {
             .setMembershipPort(gmsJoinLeaveMemberId.getMembershipPort())
             .build());
     previousMemberId.setVmViewId(0);
-    previousMemberId.getMemberData().setUUID(gmsJoinLeaveMemberId.getMemberData().getUUID());
+    previousMemberId.setUUID(gmsJoinLeaveMemberId.getUUID());
     GMSMembershipView view = new GMSMembershipView(mockMembers[0], 1,
         createMemberList(mockMembers[0], previousMemberId, mockMembers[1]));
     InstallViewMessage viewMessage = new InstallViewMessage(view, 0, false);
@@ -867,7 +867,7 @@ public void testNetworkPartitionDetected() throws Exception {
     mbrs.add(mockMembers[2]);
     mbrs.add(gmsJoinLeaveMemberId);
 
-    mockMembers[1].getMemberData().setMemberWeight((byte) 20);
+    mockMembers[1].setMemberWeight((byte) 20);
 
     GMSMembershipView newView =
         new GMSMembershipView(mockMembers[0], gmsJoinLeave.getView().getViewId() + 1, mbrs);
@@ -917,7 +917,7 @@ public void testQuorumLossNotificationWithNetworkPartitionDetectionDisabled() th
     mbrs.add(mockMembers[2]);
     mbrs.add(gmsJoinLeaveMemberId);
 
-    mockMembers[1].getMemberData().setMemberWeight((byte) 20);
+    mockMembers[1].setMemberWeight((byte) 20);
 
     GMSMembershipView newView =
         new GMSMembershipView(mockMembers[0], gmsJoinLeave.getView().getViewId() + 1, mbrs,

geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessengerJUnitTest.java

@@ -49,6 +49,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -75,9 +76,10 @@
 import org.apache.geode.distributed.internal.DistributionStats;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.distributed.internal.membership.adapter.ServiceConfig;
+import org.apache.geode.distributed.internal.membership.api.MemberData;
 import org.apache.geode.distributed.internal.membership.api.MemberDisconnectedException;
 import org.apache.geode.distributed.internal.membership.api.MemberIdentifier;
-import org.apache.geode.distributed.internal.membership.api.MemberIdentifierFactoryImpl;
+import org.apache.geode.distributed.internal.membership.api.MemberIdentifierFactory;
 import org.apache.geode.distributed.internal.membership.api.MembershipClosedException;
 import org.apache.geode.distributed.internal.membership.api.MembershipConfig;
 import org.apache.geode.distributed.internal.membership.api.Message;
@@ -186,7 +188,18 @@ private void initMocks(boolean enableMcast, Properties addProp) throws Exception
 
     // if I do this earlier then test this return messenger as null
     when(services.getMessenger()).thenReturn(messenger);
-    when(services.getMemberFactory()).thenReturn(new MemberIdentifierFactoryImpl());
+    when(services.getMemberFactory())
+        .thenReturn(new MemberIdentifierFactory<InternalDistributedMember>() {
+          @Override
+          public InternalDistributedMember create(MemberData memberInfo) {
+            return new InternalDistributedMember(memberInfo);
+          }
+
+          @Override
+          public Comparator<InternalDistributedMember> getComparator() {
+            return InternalDistributedMember::compareTo;
+          }
+        });
 
     String jgroupsConfig = messenger.jgStackConfig;
     int startIdx = jgroupsConfig.indexOf("<org");
@@ -276,7 +289,7 @@ public void testMemberWeightIsSerialized() throws Exception {
     BufferDataOutputStream out =
         new BufferDataOutputStream(500, Version.getCurrentVersion());
     MemberIdentifier mbr = createAddress(8888);
-    mbr.getMemberData().setMemberWeight((byte) 40);
+    mbr.setMemberWeight((byte) 40);
     mbr.toData(out, mock(SerializationContext.class));
     DataInputStream in = new DataInputStream(new ByteArrayInputStream(out.toByteArray()));
     mbr = new InternalDistributedMember();
@@ -553,7 +566,7 @@ public void doTestBigMessageIsFragmented(boolean mcastEnabled, boolean broadcast
     int seqno = 1;
     for (org.jgroups.Message m : messages) {
       if (jgroupsWillUseMulticast) {
-        m.setSrc(messenger.localAddress.getMemberData().getUUID());
+        m.setSrc(messenger.localAddress.getUUID());
       } else {
         m.setSrc(fakeMember);
         UNICAST3.Header oldHeader = (UNICAST3.Header) m.getHeader(unicastHeaderId);
@@ -1137,9 +1150,9 @@ public void testEncryptedJoinResponse() throws Exception {
 
   private MemberIdentifier createAddress(int port) {
     MemberIdentifier gms = new InternalDistributedMember("localhost", port);
-    gms.getMemberData().setUUID(UUID.randomUUID());
+    gms.setUUID(UUID.randomUUID());
     gms.setVmKind(MemberIdentifier.NORMAL_DM_TYPE);
-    gms.getMemberData().setVersionOrdinal(Version.getCurrentVersion().ordinal());
+    gms.setVersionObjectForTest(Version.getCurrentVersion());
     return gms;
   }
 

geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt

@@ -274,11 +274,13 @@ org/apache/geode/distributed/internal/locks/NonGrantorDestroyedProcessor$NonGran
 fromData,17
 toData,17
 
-org/apache/geode/distributed/internal/membership/InternalDistributedMember,4
-fromDataPre_GFE_7_1_0_0,7
-fromDataPre_GFE_9_0_0_0,7
-toDataPre_GFE_7_1_0_0,7
-toDataPre_GFE_9_0_0_0,7
+org/apache/geode/distributed/internal/membership/InternalDistributedMember,6
+fromData,12
+fromDataPre_GFE_7_1_0_0,12
+fromDataPre_GFE_9_0_0_0,12
+toData,12
+toDataPre_GFE_7_1_0_0,12
+toDataPre_GFE_9_0_0_0,12
 
 org/apache/geode/distributed/internal/membership/adapter/LocalViewMessage,2
 fromData,8