remove Ring access from Context

salesforce · Mar 3, 2024 · 2dab0d7 · 2dab0d7
1 parent a073856
commit 2dab0d7
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 33 deletions.
diff --git a/fireflies/src/main/java/com/salesforce/apollo/fireflies/View.java b/fireflies/src/main/java/com/salesforce/apollo/fireflies/View.java
@@ -730,12 +730,12 @@ private boolean add(AccusationWrapper accusation, Participant accuser, Participa
         if (!context.validRing(accusation.getRingNumber())) {
             return false;
         }
-        Ring<Participant> ring = context.ring(accusation.getRingNumber());
 
-        if (accused.isAccusedOn(ring.getIndex())) {
-            Participant currentAccuser = context.getMember(accused.getAccusation(ring.getIndex()).getAccuser());
+        if (accused.isAccusedOn(accusation.getRingNumber())) {
+            Participant currentAccuser = context.getMember(
+            accused.getAccusation(accusation.getRingNumber()).getAccuser());
             if (!currentAccuser.equals(accuser)) {
-                if (ring.isBetween(currentAccuser, accuser, accused)) {
+                if (context.isBetween(accusation.getRingNumber(), currentAccuser, accuser, accused)) {
                     if (!accused.verify(accusation.getSignature(),
                                         accusation.getWrapped().getAccusation().toByteString())) {
                         log.trace("Accusation discarded, accusation by: {} accused:{} signature invalid on: {}",
@@ -746,7 +746,7 @@ private boolean add(AccusationWrapper accusation, Participant accuser, Participa
                     pendingRebuttals.computeIfAbsent(accused.getId(), d -> roundTimers.schedule(() -> gc(accused),
                                                                                                 params.rebuttalTimeout()));
                     log.debug("{} accused by: {} on ring: {} (replacing: {}) on: {}", accused.getId(), accuser.getId(),
-                              ring.getIndex(), currentAccuser.getId(), node.getId());
+                              accusation.getRingNumber(), currentAccuser.getId(), node.getId());
                     if (metrics != null) {
                         metrics.accusations().mark();
                     }
@@ -769,7 +769,8 @@ private boolean add(AccusationWrapper accusation, Participant accuser, Participa
                 }
                 return false;
             }
-            Participant predecessor = ring.predecessor(accused, m -> (!m.isAccused()) || (m.equals(accuser)));
+            Participant predecessor = context.predecessor(accusation.getRingNumber(), accused,
+                                                          m -> (!m.isAccused()) || (m.equals(accuser)));
             if (accuser.equals(predecessor)) {
                 accused.addAccusation(accusation);
                 if (!accused.equals(node) && !pendingRebuttals.containsKey(accused.getId())) {
@@ -1937,7 +1938,7 @@ public Gossip rumors(SayWhat request, Digest from) {
                     }
                 }
 
-                Participant successor = context.ring(ring).successor(member, m -> context.isActive(m.getId()));
+                Participant successor = context.successor(ring, member, m -> context.isActive(m.getId()));
                 if (successor == null) {
                     log.debug("No active successor on ring: {} from: {} on: {}", ring, from, node.getId());
                     throw new StatusRuntimeException(Status.FAILED_PRECONDITION.withDescription(
@@ -1999,7 +2000,7 @@ public void update(State request, Digest from) {
                     Status.INVALID_ARGUMENT.withDescription("No successor of: " + from));
                 }
                 Participant member = context.getActiveMember(from);
-                Participant successor = context.ring(ring).successor(member, m -> context.isActive(m.getId()));
+                Participant successor = context.successor(ring, member, m -> context.isActive(m.getId()));
                 if (successor == null) {
                     log.debug("No successor, invalid update from: {} on ring: {} on: {}", from, ring, node.getId());
                     throw new StatusRuntimeException(

diff --git a/fireflies/src/test/java/com/salesforce/apollo/fireflies/ChurnTest.java b/fireflies/src/test/java/com/salesforce/apollo/fireflies/ChurnTest.java
@@ -240,7 +240,7 @@ public void churn() throws Exception {
         for (View v : views) {
             Graph<Participant> testGraph = new Graph<>();
             for (int i = 0; i < v.getContext().getRingCount(); i++) {
-                testGraph.addEdge(v.getNode(), v.getContext().ring(i).successor(v.getNode()));
+                testGraph.addEdge(v.getNode(), v.getContext().successor(i, v.getNode()));
             }
             assertTrue(testGraph.isSC());
         }

diff --git a/fireflies/src/test/java/com/salesforce/apollo/fireflies/E2ETest.java b/fireflies/src/test/java/com/salesforce/apollo/fireflies/E2ETest.java
@@ -223,10 +223,10 @@ private void post() {
     }
 
     private void validateConstraints() {
+        final var reference = views.get(0).getContext().stream(0).collect(Collectors.toSet());
         for (int i = 0; i < views.get(0).getContext().getRingCount(); i++) {
-            final var reference = views.get(0).getContext().ring(i).getRing();
             for (View view : views) {
-                assertEquals(reference, view.getContext().ring(i).getRing());
+                assertTrue(reference.containsAll(view.getContext().stream(i).toList()));
             }
         }
 
@@ -241,7 +241,7 @@ private void validateConstraints() {
         for (View v : views) {
             Graph<Participant> testGraph = new Graph<>();
             for (int i = 0; i < views.get(0).getContext().getRingCount(); i++) {
-                testGraph.addEdge(v.getNode(), v.getContext().ring(i).successor(v.getNode()));
+                testGraph.addEdge(v.getNode(), v.getContext().successor(i, v.getNode()));
             }
             assertTrue(testGraph.isSC());
         }

diff --git a/fireflies/src/test/java/com/salesforce/apollo/fireflies/SwarmTest.java b/fireflies/src/test/java/com/salesforce/apollo/fireflies/SwarmTest.java
@@ -163,10 +163,10 @@ public void swarm() throws Exception {
         + " members");
 
         if (!largeTests) {
+            final var reference = views.get(0).getContext().stream(0).collect(Collectors.toSet());
             for (int i = 0; i < views.get(0).getContext().getRingCount(); i++) {
-                final var reference = views.get(0).getContext().ring(i).getRing();
                 for (View view : views) {
-                    assertEquals(reference, view.getContext().ring(i).getRing());
+                    assertTrue(reference.containsAll(view.getContext().stream(i).toList()));
                 }
             }
 
@@ -180,7 +180,7 @@ public void swarm() throws Exception {
             for (View v : views) {
                 Graph<Participant> testGraph = new Graph<>();
                 for (int i = 0; i < views.get(0).getContext().getRingCount(); i++) {
-                    testGraph.addEdge(v.getNode(), v.getContext().ring(i).successor(v.getNode()));
+                    testGraph.addEdge(v.getNode(), v.getContext().successor(i, v.getNode()));
                 }
                 assertTrue(testGraph.isSC());
             }

diff --git a/memberships/src/main/java/com/salesforce/apollo/context/Context.java b/memberships/src/main/java/com/salesforce/apollo/context/Context.java
@@ -362,6 +362,14 @@ default int majority() {
      */
     int size();
 
+    /**
+     * Stream the members of the ring in hashed order
+     *
+     * @param ring
+     * @return
+     */
+    Stream<T> stream(int ring);
+
     /**
      * @param ring
      * @param predicate

diff --git a/memberships/src/main/java/com/salesforce/apollo/context/DynamicContext.java b/memberships/src/main/java/com/salesforce/apollo/context/DynamicContext.java
@@ -180,11 +180,6 @@ public DynamicContext<Z> build() {
      */
     void remove(T m);
 
-    /**
-     * @return the indexed Ring<T>
-     */
-    Ring<T> ring(int index);
-
     /**
      * @return the Stream of rings managed by the context
      */

diff --git a/memberships/src/main/java/com/salesforce/apollo/context/DynamicContextImpl.java b/memberships/src/main/java/com/salesforce/apollo/context/DynamicContextImpl.java
@@ -577,17 +577,6 @@ public void remove(T m) {
         remove(m.getId());
     }
 
-    /**
-     * @return the indexed Ring<T>
-     */
-    @Override
-    public Ring<T> ring(int index) {
-        if (index < 0 || index >= rings.size()) {
-            throw new IllegalArgumentException("Not a valid ring #: " + index + " max: " + (rings.size() - 1));
-        }
-        return rings.get(index);
-    }
-
     /**
      * @return the Stream of rings managed by the context
      */
@@ -632,6 +621,11 @@ public int size() {
         return members.size();
     }
 
+    @Override
+    public Stream<T> stream(int ring) {
+        return ring(ring).stream();
+    }
+
     @Override
     public Stream<T> streamPredecessors(int ring, Digest location, Predicate<T> predicate) {
         return ring(ring).streamPredecessors(location, predicate);
@@ -780,6 +774,16 @@ private Digest[] hashesFor(T m) {
         return s;
     }
 
+    /**
+     * @return the indexed Ring<T>
+     */
+    private Ring<T> ring(int index) {
+        if (index < 0 || index >= rings.size()) {
+            throw new IllegalArgumentException("Not a valid ring #: " + index + " max: " + (rings.size() - 1));
+        }
+        return rings.get(index);
+    }
+
     private Tracked<T> tracking(T m) {
         return members.computeIfAbsent(m.getId(), id1 -> {
             for (var ring : rings) {

diff --git a/memberships/src/main/java/com/salesforce/apollo/context/StaticContext.java b/memberships/src/main/java/com/salesforce/apollo/context/StaticContext.java
@@ -275,6 +275,11 @@ public int size() {
         return 0;
     }
 
+    @Override
+    public Stream<T> stream(int ring) {
+        return null;
+    }
+
     @Override
     public Stream<T> streamPredecessors(int ring, Digest location, Predicate<T> predicate) {
         return null;

diff --git a/memberships/src/test/java/com/salesforce/apollo/context/CompactRingTest.java b/memberships/src/test/java/com/salesforce/apollo/context/CompactRingTest.java
@@ -41,7 +41,7 @@ public void smokin() throws Exception {
         assertEquals(context.getRingCount(), compact.getRingCount());
 
         for (int i = 0; i < context.getRingCount(); i++) {
-            assertEquals(context.ring(i).stream().map(m -> m.getId()).toList(), compact.ring(i).stream().toList(),
+            assertEquals(context.stream(i).map(m -> m.getId()).toList(), compact.ring(i).stream().toList(),
                          "Ring " + i + " mismatched");
         }
     }

diff --git a/memberships/src/test/java/com/salesforce/apollo/context/ContextTests.java b/memberships/src/test/java/com/salesforce/apollo/context/ContextTests.java
@@ -45,6 +45,6 @@ public void consistency() throws Exception {
 
         List<Member> successors = context.successors(members.get(1));
         assertEquals(members.get(0), successors.get(0));
-        assertEquals(members.get(1), context.ring(1).successor(members.get(0)));
+        assertEquals(members.get(1), context.successor(1, members.get(0)));
     }
 }