From a55e94ccc94598c6fcafe65816e384018a85fc45 Mon Sep 17 00:00:00 2001 From: Mark McLaughlin Date: Thu, 22 Aug 2024 16:08:09 +0100 Subject: [PATCH] Move generic tests from RelationsGrpcClientsManagerTest to ChannelManagerTest. --- .../clients/KesselClientsManager.java | 14 -- .../clients/ChannelManagerTest.java | 166 +++++++++++++++++- .../RelationsGrpcClientsManagerTest.java | 156 ---------------- 3 files changed, 163 insertions(+), 173 deletions(-) diff --git a/src/main/java/org/project_kessel/clients/KesselClientsManager.java b/src/main/java/org/project_kessel/clients/KesselClientsManager.java index 17f3d59..d79f4e4 100644 --- a/src/main/java/org/project_kessel/clients/KesselClientsManager.java +++ b/src/main/java/org/project_kessel/clients/KesselClientsManager.java @@ -8,18 +8,4 @@ public abstract class KesselClientsManager { protected KesselClientsManager(Channel channel) { this.channel = channel; } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - KesselClientsManager that = (KesselClientsManager) o; - return channel.equals(that.channel); - } - - @Override - public int hashCode() { - return channel.hashCode(); - } } diff --git a/src/test/java/org/project_kessel/clients/ChannelManagerTest.java b/src/test/java/org/project_kessel/clients/ChannelManagerTest.java index e2a5e26..de15e00 100644 --- a/src/test/java/org/project_kessel/clients/ChannelManagerTest.java +++ b/src/test/java/org/project_kessel/clients/ChannelManagerTest.java @@ -1,11 +1,27 @@ package org.project_kessel.clients; +import io.grpc.Channel; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; +import java.util.HashMap; +import java.util.Hashtable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import static io.smallrye.common.constraint.Assert.assertNotNull; +import static org.junit.jupiter.api.Assertions.*; + +class ChannelManagerTest { + ChannelManager defaultChannelManager = ChannelManager.getInstance("defaultChannelManager"); + + @AfterEach + void testTeardown() { + /* Make sure all client managers shutdown/removed after each test */ + defaultChannelManager.shutdownAll(); + } -public class ChannelManagerTest { @Test void shouldSelectSameManager() { ChannelManager channelManager = ChannelManager.getInstance("myChannelManangerKey"); @@ -21,4 +37,148 @@ void shouldNotSelectSameManager() { assertNotEquals(channelManager, channelManager2); } + + @Test + void testManagerChannelReusePatterns() { + var one = defaultChannelManager.forInsecureClients("localhost:8080"); + var two = defaultChannelManager.forInsecureClients("localhost:8080"); // same as one + var three = defaultChannelManager.forInsecureClients("localhost1:8080"); + var four = defaultChannelManager.forSecureClients("localhost:8080"); + var five = defaultChannelManager.forSecureClients("localhost1:8080"); + var six = defaultChannelManager.forSecureClients("localhost1:8080"); // same as five + + assertNotNull(one); + assertNotNull(two); + assertNotNull(three); + assertNotNull(four); + assertNotNull(five); + assertNotNull(six); + assertEquals(one, two); + assertNotEquals(two, three); + assertEquals(five, six); + assertNotEquals(four, five); + } + + @Test + void testThreadingChaos() { + /* Basic testing to ensure that we don't get ConcurrentModificationExceptions, or any other exceptions, when + * creating and destroying managers on different threads. */ + + try { + Hashtable channels = new Hashtable<>(); + + int numberOfThreads = 100; + ExecutorService service = Executors.newFixedThreadPool(numberOfThreads); + CountDownLatch latch1 = new CountDownLatch(numberOfThreads / 3); + CountDownLatch latch2 = new CountDownLatch(numberOfThreads * 2 / 3 - numberOfThreads / 3); + CountDownLatch latch3 = new CountDownLatch(numberOfThreads - numberOfThreads * 2 / 3); + + /* A: Use 1/3 of threads to request/create channels at the same time. */ + for (int i = 0; i < numberOfThreads / 3; i++) { + final int j = i; + service.submit(() -> { + Channel manager; + if(j % 2 == 0) { + manager = defaultChannelManager.forInsecureClients("localhost" + j); + } else { + manager = defaultChannelManager.forSecureClients("localhost" + j); + } + channels.put("localhost" + j, manager); + + latch1.countDown(); + }); + } + + latch1.await(); + /* B and C, below, trigger at the same time once A is done. */ + + /* B: Use 1/3 of threads to shut down the above created channels. */ + for (int i = numberOfThreads / 3; i < numberOfThreads * 2 / 3; i++) { + final int j = i - numberOfThreads / 3; + service.submit(() -> { + defaultChannelManager.shutdownChannel(channels.get("localhost" + j)); + latch2.countDown(); + }); + } + + /* C: Use 1/3 of the threads to recreate/retrieve the same channels at the same time as B. */ + for (int i = numberOfThreads * 2 / 3; i < numberOfThreads; i++) { + final int j = i - numberOfThreads * 2 / 3; + service.submit(() -> { + Channel manager; + if(j % 2 == 0) { + manager = defaultChannelManager.forInsecureClients("localhost" + j); + } else { + manager = defaultChannelManager.forSecureClients("localhost" + j); + } + channels.put("localhost" + j, manager); + + latch3.countDown(); + }); + } + latch2.await(); + latch3.await(); + } catch(Exception e) { + fail("Should not have thrown any exception"); + } + } + + /* + Tests relying on reflection. Brittle and could be removed in future. + */ + + @Test + void testManagerReuseInternal() throws Exception { + defaultChannelManager.forInsecureClients("localhost:8080"); + defaultChannelManager.forInsecureClients("localhost:8080"); // same as one + defaultChannelManager.forInsecureClients("localhost1:8080"); + defaultChannelManager.forSecureClients("localhost:8080"); + defaultChannelManager.forSecureClients("localhost1:8080"); + defaultChannelManager.forSecureClients("localhost1:8080"); // same as five + + var insecureField = ChannelManager.class.getDeclaredField("insecureManagers"); + insecureField.setAccessible(true); + var secureField = ChannelManager.class.getDeclaredField("secureManagers"); + secureField.setAccessible(true); + var insecureManagers = (HashMap)insecureField.get(defaultChannelManager); + var secureManagers = (HashMap)secureField.get(defaultChannelManager); + + assertEquals(2, insecureManagers.size()); + assertEquals(2, secureManagers.size()); + } + + @Test + void testCreateAndShutdownPatternsInternal() throws Exception { + var insecureField = ChannelManager.class.getDeclaredField("insecureManagers"); + insecureField.setAccessible(true); + var insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + + assertEquals(0, insecureManagersSize); + + var channel = defaultChannelManager.forInsecureClients("localhost:8080"); + insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + assertEquals(1, insecureManagersSize); + + defaultChannelManager.shutdownChannel(channel); + insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + assertEquals(0, insecureManagersSize); + + /* Shouldn't throw exception if executed twice */ + defaultChannelManager.shutdownChannel(channel); + insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + assertEquals(0, insecureManagersSize); + + var manager2 = defaultChannelManager.forInsecureClients("localhost:8080"); + insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + assertEquals(1, insecureManagersSize); + assertNotEquals(channel, manager2); + + defaultChannelManager.forInsecureClients("localhost:8081"); + insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + assertEquals(2, insecureManagersSize); + + defaultChannelManager.shutdownAll(); + insecureManagersSize = ((HashMap)insecureField.get(defaultChannelManager)).size(); + assertEquals(0, insecureManagersSize); + } } diff --git a/src/test/java/org/project_kessel/relations/client/RelationsGrpcClientsManagerTest.java b/src/test/java/org/project_kessel/relations/client/RelationsGrpcClientsManagerTest.java index 4691b8f..9eaca41 100644 --- a/src/test/java/org/project_kessel/relations/client/RelationsGrpcClientsManagerTest.java +++ b/src/test/java/org/project_kessel/relations/client/RelationsGrpcClientsManagerTest.java @@ -9,17 +9,11 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.project_kessel.clients.ChannelManager; import org.project_kessel.clients.KesselClient; import org.project_kessel.clients.authn.AuthenticationConfig; import org.project_kessel.clients.fake.GrpcServerSpy; -import java.util.HashMap; -import java.util.Hashtable; import java.util.Optional; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import static io.smallrye.common.constraint.Assert.assertNotNull; import static org.junit.jupiter.api.Assertions.*; @@ -48,91 +42,6 @@ static void removeTestSetup() { removeTestCACertFromKeystore(); } - @Test - void testManagerChannelReusePatterns() { - var one = RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); - var two = RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); // same as one - var three = RelationsGrpcClientsManager.forInsecureClients("localhost1:8080"); - var four = RelationsGrpcClientsManager.forSecureClients("localhost:8080"); - var five = RelationsGrpcClientsManager.forSecureClients("localhost1:8080"); - var six = RelationsGrpcClientsManager.forSecureClients("localhost1:8080"); // same as five - - assertNotNull(one); - assertNotNull(two); - assertNotNull(three); - assertNotNull(four); - assertNotNull(five); - assertNotNull(six); - assertEquals(one, two); - assertNotEquals(two, three); - assertEquals(five, six); - assertNotEquals(four, five); - } - - @Test - void testThreadingChaos() { - /* Basic testing to ensure that we don't get ConcurrentModificationExceptions, or any other exceptions, when - * creating and destroying managers on different threads. */ - - try { - Hashtable managers = new Hashtable<>(); - - int numberOfThreads = 100; - ExecutorService service = Executors.newFixedThreadPool(numberOfThreads); - CountDownLatch latch1 = new CountDownLatch(numberOfThreads / 3); - CountDownLatch latch2 = new CountDownLatch(numberOfThreads * 2 / 3 - numberOfThreads / 3); - CountDownLatch latch3 = new CountDownLatch(numberOfThreads - numberOfThreads * 2 / 3); - - /* A: Use 1/3 of threads to request/create managers at the same time. */ - for (int i = 0; i < numberOfThreads / 3; i++) { - final int j = i; - service.submit(() -> { - RelationsGrpcClientsManager manager; - if(j % 2 == 0) { - manager = RelationsGrpcClientsManager.forInsecureClients("localhost" + j); - } else { - manager = RelationsGrpcClientsManager.forSecureClients("localhost" + j); - } - managers.put("localhost" + j, manager); - - latch1.countDown(); - }); - } - - latch1.await(); - /* B and C, below, trigger at the same time once A is done. */ - - /* B: Use 1/3 of threads to shut down the above created managers. */ - for (int i = numberOfThreads / 3; i < numberOfThreads * 2 / 3; i++) { - final int j = i - numberOfThreads / 3; - service.submit(() -> { - RelationsGrpcClientsManager.shutdownManager(managers.get("localhost" + j)); - latch2.countDown(); - }); - } - - /* C: Use 1/3 of the threads to recreate/retrieve the same managers at the same time as B. */ - for (int i = numberOfThreads * 2 / 3; i < numberOfThreads; i++) { - final int j = i - numberOfThreads * 2 / 3; - service.submit(() -> { - RelationsGrpcClientsManager manager; - if(j % 2 == 0) { - manager = RelationsGrpcClientsManager.forInsecureClients("localhost" + j); - } else { - manager = RelationsGrpcClientsManager.forSecureClients("localhost" + j); - } - managers.put("localhost" + j, manager); - - latch3.countDown(); - }); - } - latch2.await(); - latch3.await(); - } catch(Exception e) { - fail("Should not have thrown any exception"); - } - } - /* End-to-end tests against fake IdP and/or fake grpc relations-api */ @@ -172,31 +81,6 @@ void testManagersHoldIntendedCredentialsInChannel() throws Exception { Tests relying on reflection. Brittle and could be removed in future. */ - @Test - void testManagerReuseInternal() throws Exception { - RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); - RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); // same as one - RelationsGrpcClientsManager.forInsecureClients("localhost1:8080"); - RelationsGrpcClientsManager.forSecureClients("localhost:8080"); - RelationsGrpcClientsManager.forSecureClients("localhost1:8080"); - RelationsGrpcClientsManager.forSecureClients("localhost1:8080"); // same as five - - var channelManagerKeyField = RelationsGrpcClientsManager.class.getDeclaredField("CHANNEL_MANAGER_KEY"); - channelManagerKeyField.setAccessible(true); - var channelManagerKey = (String)channelManagerKeyField.get(null); - var kesselChannelManager = ChannelManager.getInstance(channelManagerKey); - - var insecureField = ChannelManager.class.getDeclaredField("insecureManagers"); - insecureField.setAccessible(true); - var secureField = ChannelManager.class.getDeclaredField("secureManagers"); - secureField.setAccessible(true); - var insecureManagers = (HashMap)insecureField.get(kesselChannelManager); - var secureManagers = (HashMap)secureField.get(kesselChannelManager); - - assertEquals(2, insecureManagers.size()); - assertEquals(2, secureManagers.size()); - } - @Test void testSameChannelUsedByClientsInternal() throws Exception { var manager = RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); @@ -218,46 +102,6 @@ void testSameChannelUsedByClientsInternal() throws Exception { assertEquals(lookupChannel, relationTuplesChannel); } - @Test - void testCreateAndShutdownPatternsInternal() throws Exception { - var channelManagerKeyField = RelationsGrpcClientsManager.class.getDeclaredField("CHANNEL_MANAGER_KEY"); - channelManagerKeyField.setAccessible(true); - var channelManagerKey = (String)channelManagerKeyField.get(null); - var kesselChannelManager = ChannelManager.getInstance(channelManagerKey); - - var insecureField = ChannelManager.class.getDeclaredField("insecureManagers"); - insecureField.setAccessible(true); - var insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - - assertEquals(0, insecureManagersSize); - - var manager = RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); - insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - assertEquals(1, insecureManagersSize); - - RelationsGrpcClientsManager.shutdownManager(manager); - insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - assertEquals(0, insecureManagersSize); - - /* Shouldn't throw exception if executed twice */ - RelationsGrpcClientsManager.shutdownManager(manager); - insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - assertEquals(0, insecureManagersSize); - - var manager2 = RelationsGrpcClientsManager.forInsecureClients("localhost:8080"); - insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - assertEquals(1, insecureManagersSize); - assertNotEquals(manager, manager2); - - RelationsGrpcClientsManager.forInsecureClients("localhost:8081"); - insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - assertEquals(2, insecureManagersSize); - - RelationsGrpcClientsManager.shutdownAll(); - insecureManagersSize = ((HashMap)insecureField.get(kesselChannelManager)).size(); - assertEquals(0, insecureManagersSize); - } - public static Config.AuthenticationConfig dummyAuthConfigWithGoodOIDCClientCredentials() { return new Config.AuthenticationConfig() { @Override