diff --git a/README.md b/README.md index b39a86f8..cc58e489 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,9 @@ Per the above setup steps, the UI runs on `http://localhost:8089/`. To see your ## V2 API -The v2 API is based on individual route provider classes. Each class should provide exactly one endpoint and must implement IRouteProvider or IBlockingRouteProvider. +The v2 API is based on individual route provider classes. Each class should provide exactly one endpoint and must implement IRouteProvider or IBlockingRouteProvider. It may accept constructor parameters, which will be auto-wired by our DI system. Currently, DI is configured to provide: +- All the IService classes which are provided to the Admin Verticle. +- The Auth middleware (but see IRouteProvider - you probably don't need it). ### IRouteProvider @@ -30,4 +32,12 @@ The v2 API is based on individual route provider classes. Each class should prov IRouteProvider requires a `getHandler` method, which should return a valid handler function - see `GetClientSideKeypairsBySite.java`. This method *must* be annotated with the Path, Method, and Roles annotations. -The route handler will automatically be wrapped by the Auth middleware based on the roles specified in the Roles annotation. +All classes which implement IRouteProvider will automatically be picked up by DI and registered as route handlers. The route handler will automatically be wrapped by the Auth middleware based on the roles specified in the Roles annotation. + +Currently, we require the explicit `@Inject` annotation on all constructors which are valid for the DI framework to use. Your IRouteProvider implementation *must* have a constructor with the @Inject annotation. + +## Dependency injection - current state and plans + +We are in the process of introducing dependency injection to the code base. Currently, a number of singletons which are constructed explicitly are provided via `ServicesModule` (for `IService` classes) and the `SingletonsModule` (for other singletons - e.g. the Auth middleware). + +Over time, it would be nice to expand what is being constructed via DI and reduce our reliance on manually constructing objects. Once we have all of the dependencies for `AdminVerticle` available via DI, we can stop creating the `V2Router` via DI and instead just create the `AdminVerticle` (DI will then create the `V2Router` for us). \ No newline at end of file diff --git a/pom.xml b/pom.xml index 9cb4303b..c4fd4e96 100644 --- a/pom.xml +++ b/pom.xml @@ -68,6 +68,11 @@ vertx-auth-oauth2 ${vertx.version} + + com.google.inject + guice + 7.0.0 + io.micrometer micrometer-registry-jmx diff --git a/src/main/java/com/uid2/admin/Main.java b/src/main/java/com/uid2/admin/Main.java index bb430428..51318526 100644 --- a/src/main/java/com/uid2/admin/Main.java +++ b/src/main/java/com/uid2/admin/Main.java @@ -1,6 +1,8 @@ package com.uid2.admin; import com.fasterxml.jackson.databind.ObjectWriter; +import com.google.inject.Guice; +import com.google.inject.Injector; import com.uid2.admin.auth.AdminUserProvider; import com.uid2.admin.auth.GithubAuthFactory; import com.uid2.admin.auth.AuthFactory; @@ -21,6 +23,7 @@ import com.uid2.admin.vertx.AdminVerticle; import com.uid2.admin.vertx.JsonUtil; import com.uid2.admin.vertx.WriteLock; +import com.uid2.admin.vertx.api.V2Router; import com.uid2.admin.vertx.api.V2RouterModule; import com.uid2.admin.vertx.service.*; import com.uid2.shared.Const; @@ -260,9 +263,27 @@ public void run() { "admins", 10000, adminUserProvider); vertx.deployVerticle(rotatingAdminUserStoreVerticle); - val v2RouterModule = new V2RouterModule(clientSideKeypairService, auth); + /* + Begin introducing dependency injection - for now, it just knows about: + - all of the IService classes + - v2 API handlers + - authHandler + N.b. there should only ever be one injector! + */ + Injector injector = Guice.createInjector( + new RequireInjectAnnotationsModule(), + new ServicesModule(services), + new SingletonsModule(auth), + new V2RouterModule() + ); + /* + Grab the V2 API route provider. N.b. there should usually only be a single call to injector. + The next step is probably to get Guice to construct the Admin verticle instead of the v2 router - + but we'll need to get the Admin Verticle's other dependencies managed by Guice first. + */ + val v2Api = injector.getInstance(V2Router.class); - AdminVerticle adminVerticle = new AdminVerticle(config, authFactory, adminUserProvider, services, v2RouterModule.getRouter()); + AdminVerticle adminVerticle = new AdminVerticle(config, authFactory, adminUserProvider, services, v2Api); vertx.deployVerticle(adminVerticle); CloudPath keysetMetadataPath = new CloudPath(config.getString("keysets_metadata_path")); diff --git a/src/main/java/com/uid2/admin/RequireInjectAnnotationsModule.java b/src/main/java/com/uid2/admin/RequireInjectAnnotationsModule.java new file mode 100644 index 00000000..55cb7853 --- /dev/null +++ b/src/main/java/com/uid2/admin/RequireInjectAnnotationsModule.java @@ -0,0 +1,16 @@ +package com.uid2.admin; + +import com.google.inject.AbstractModule; + +/* + This is used as part of the gradual introduction of DI within the codebase to ensure Google Guice doesn't try + to instantiate anything that hasn't been marked as available for automated construction. + Eventually we probably won't want this, but this helps ensure a staged DI adoption doesn't do anything unexpected. +*/ +public class RequireInjectAnnotationsModule extends AbstractModule { + @Override + protected void configure() { + // Prevent Guice from using any constructors which haven't been marked with the @Inject attribute + binder().requireAtInjectOnConstructors(); + } +} diff --git a/src/main/java/com/uid2/admin/SingletonsModule.java b/src/main/java/com/uid2/admin/SingletonsModule.java new file mode 100644 index 00000000..85c95808 --- /dev/null +++ b/src/main/java/com/uid2/admin/SingletonsModule.java @@ -0,0 +1,27 @@ +package com.uid2.admin; + +import com.google.inject.AbstractModule; +import com.uid2.admin.vertx.service.IService; + +import java.util.Arrays; + +/* + This is a temporary module which accepts an array of pre-created singletons and makes them available as a module. + Over time, we would ideally move to letting the DI framework create the singletons as well - this temporary solution + is being used to support a strangler-pattern introduction of DI. +*/ +public class SingletonsModule extends AbstractModule { + private final Object[] singletons; + + public SingletonsModule(Object... singletons) { + this.singletons = singletons; + } + + @Override + protected void configure() { + super.configure(); + Arrays.stream(singletons).forEach(s -> { + bind((Class)s.getClass()).toInstance(s); + }); + } +} diff --git a/src/main/java/com/uid2/admin/vertx/api/IRouteProvider.java b/src/main/java/com/uid2/admin/vertx/api/IRouteProvider.java index 32f6503f..62d74020 100644 --- a/src/main/java/com/uid2/admin/vertx/api/IRouteProvider.java +++ b/src/main/java/com/uid2/admin/vertx/api/IRouteProvider.java @@ -5,6 +5,10 @@ import io.vertx.ext.web.RoutingContext; /* + Implement this interface to automatically be picked up by V2Router and have your routes registered under /v2api/. + Any constructor dependencies which are registered should be auto-injected by Guice, as long as it knows about them. + You must have a constructor marked with @Inject for DI to use it. + *Important* If you implement this interface, your route will be registered as a non-blocking handler. Use IBlockingRouteProvider instead if you want to provide a blocking handler. See `readme.md` for more information. diff --git a/src/main/java/com/uid2/admin/vertx/api/V2Router.java b/src/main/java/com/uid2/admin/vertx/api/V2Router.java index 8f238132..066cb874 100644 --- a/src/main/java/com/uid2/admin/vertx/api/V2Router.java +++ b/src/main/java/com/uid2/admin/vertx/api/V2Router.java @@ -1,11 +1,14 @@ package com.uid2.admin.vertx.api; +import com.google.inject.Inject; +import com.google.inject.Singleton; import com.uid2.admin.vertx.api.annotations.Method; import com.uid2.admin.vertx.api.annotations.Path; import com.uid2.admin.vertx.api.annotations.Roles; import com.uid2.shared.middleware.AuthMiddleware; import io.vertx.core.Vertx; import io.vertx.ext.web.Router; +import io.vertx.ext.web.RoutingContext; import lombok.val; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -13,12 +16,14 @@ import java.util.Arrays; import java.util.Set; +@Singleton public class V2Router { private static final Logger LOGGER = LoggerFactory.getLogger(V2Router.class); - private final IRouteProvider[] routeProviders; + private final Set routeProviders; private final AuthMiddleware auth; - public V2Router(IRouteProvider[] routeProviders, AuthMiddleware auth) { + @Inject + public V2Router(Set routeProviders, AuthMiddleware auth) { this.routeProviders = routeProviders; this.auth = auth; } diff --git a/src/main/java/com/uid2/admin/vertx/api/V2RouterModule.java b/src/main/java/com/uid2/admin/vertx/api/V2RouterModule.java index 63ff8a09..e975401e 100644 --- a/src/main/java/com/uid2/admin/vertx/api/V2RouterModule.java +++ b/src/main/java/com/uid2/admin/vertx/api/V2RouterModule.java @@ -1,29 +1,47 @@ package com.uid2.admin.vertx.api; -import com.uid2.admin.secret.IKeypairManager; -import com.uid2.admin.vertx.api.cstg.GetClientSideKeypairsBySite; -import com.uid2.shared.middleware.AuthMiddleware; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.reflect.ClassPath; +import com.google.inject.AbstractModule; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import com.google.inject.multibindings.Multibinder; +import lombok.val; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class V2RouterModule { - private static final Logger LOGGER = LoggerFactory.getLogger(V2RouterModule.class); - - private final IKeypairManager keypairManager; - private final AuthMiddleware authMiddleware; +import java.io.IOException; +import java.util.Arrays; +import java.util.stream.Collectors; - public V2RouterModule(IKeypairManager keypairManager, AuthMiddleware authMiddleware) { - this.keypairManager = keypairManager; - this.authMiddleware = authMiddleware; - } +public class V2RouterModule extends AbstractModule { + private static final Logger LOGGER = LoggerFactory.getLogger(V2RouterModule.class); - protected IRouteProvider[] getRouteProviders() { - return new IRouteProvider[] { - new GetClientSideKeypairsBySite(keypairManager) - }; - } + /* + Finds all classes in com.uid2.admin.vertx.api which implement IRouterProvider and register them. + They are registered both as IRouterProvider and as their individual class. + */ + @Override + protected void configure() { + try { + Multibinder interfaceBinder = Multibinder.newSetBinder(binder(), IRouteProvider.class); - public V2Router getRouter() { - return new V2Router(getRouteProviders(), authMiddleware); + val cp = ClassPath.from(getClass().getClassLoader()); + val routerProviders = cp + .getTopLevelClasses() + .stream() + .filter(ci -> ci.getName().startsWith("com.uid2.admin.vertx.api")) + .map(ci -> ci.load()) + .filter(cl -> !cl.isInterface() && Arrays.stream(cl.getInterfaces()).anyMatch(interf -> interf == IRouteProvider.class || interf == IBlockingRouteProvider.class)) + .map(cl -> (Class)cl) + .collect(Collectors.toSet()); + for (val routerProviderClass : routerProviders) { + LOGGER.info("Registering v2 route provider " + routerProviderClass.getName()); + bind(routerProviderClass); + interfaceBinder.addBinding().to(routerProviderClass); + } + } catch (IOException e) { + throw new RuntimeException(e); + } } } diff --git a/src/main/java/com/uid2/admin/vertx/api/cstg/GetClientSideKeypairsBySite.java b/src/main/java/com/uid2/admin/vertx/api/cstg/GetClientSideKeypairsBySite.java index 1384e0bb..93a03b13 100644 --- a/src/main/java/com/uid2/admin/vertx/api/cstg/GetClientSideKeypairsBySite.java +++ b/src/main/java/com/uid2/admin/vertx/api/cstg/GetClientSideKeypairsBySite.java @@ -1,6 +1,7 @@ package com.uid2.admin.vertx.api.cstg; import com.google.common.collect.Streams; +import com.google.inject.Inject; import com.uid2.admin.secret.IKeypairManager; import com.uid2.admin.vertx.ResponseUtil; import com.uid2.admin.vertx.api.IRouteProvider; @@ -9,6 +10,7 @@ import com.uid2.admin.vertx.api.annotations.Method; import com.uid2.admin.vertx.api.annotations.Path; import com.uid2.admin.vertx.api.annotations.Roles; +import com.uid2.admin.vertx.service.ClientSideKeypairService; import com.uid2.shared.auth.Role; import io.vertx.core.Handler; import io.vertx.ext.web.RoutingContext; @@ -21,6 +23,8 @@ public class GetClientSideKeypairsBySite implements IRouteProvider { private final IKeypairManager keypairManager; + + @Inject public GetClientSideKeypairsBySite(IKeypairManager keypairManager) { this.keypairManager = keypairManager; } diff --git a/src/main/java/com/uid2/admin/vertx/service/ServicesModule.java b/src/main/java/com/uid2/admin/vertx/service/ServicesModule.java new file mode 100644 index 00000000..0dc0b7ae --- /dev/null +++ b/src/main/java/com/uid2/admin/vertx/service/ServicesModule.java @@ -0,0 +1,31 @@ +package com.uid2.admin.vertx.service; + +import com.google.inject.AbstractModule; +import com.google.inject.multibindings.Multibinder; +import lombok.val; + +import java.util.Arrays; + +/* + This is a temporary module which accepts an array of pre-created singleton services and makes them available as a module. + Over time, we would ideally move to letting the DI framework create the services as well - this temporary solution + is being used to support a strangler-pattern introduction of DI. +*/ +public class ServicesModule extends AbstractModule { + private final IService[] services; + + public ServicesModule(IService[] services) { + this.services = services; + } + + @Override + protected void configure() { + Multibinder serviceInterfaceBinder = Multibinder.newSetBinder(binder(), IService.class); + Arrays.stream(services).forEach(s -> { + bind((Class)s.getClass()).toInstance(s); + serviceInterfaceBinder.addBinding().toInstance(s); + val interfaces = Arrays.stream(s.getClass().getInterfaces()).filter(i -> i != IService.class); + interfaces.forEach(i -> bind((Class)i).toInstance(s)); + }); + } +} diff --git a/src/test/java/com/uid2/admin/GuiceMockInjectingModule.java b/src/test/java/com/uid2/admin/GuiceMockInjectingModule.java new file mode 100644 index 00000000..f2873634 --- /dev/null +++ b/src/test/java/com/uid2/admin/GuiceMockInjectingModule.java @@ -0,0 +1,36 @@ +package com.uid2.admin; + +import com.google.inject.AbstractModule; +import com.google.inject.multibindings.Multibinder; +import com.uid2.admin.vertx.service.IService; +import lombok.val; + +import java.io.InvalidClassException; +import java.util.Arrays; +import java.util.stream.Collectors; + +import static org.mockito.Mockito.*; + +public class GuiceMockInjectingModule extends AbstractModule { + private final Object[] mocks; + + public GuiceMockInjectingModule(Object... mocks) throws InvalidClassException { + for (Object mock : mocks) { + val mockDetails = mockingDetails(mock); + if (!mockDetails.isMock()) throw new InvalidClassException( + "GuiceMockInjectingModule is for injecting mocks, but found an object which was not a mock:" + mockDetails.getClass().getName() + ); + } + this.mocks = mocks; + } + + @Override + protected void configure() { + Arrays.stream(mocks).forEach(mock -> { + System.out.println("Configuring mock for class " + mock.getClass().getName()); + bind((Class)mock.getClass()).toInstance(mock); + val interfaces = Arrays.stream(mock.getClass().getInterfaces()).filter(iface -> iface != IService.class); + interfaces.forEach(iface -> bind((Class)iface).toInstance(mock)); + }); + } +} diff --git a/src/test/java/com/uid2/admin/v2Router/RouterConfigurationTests.java b/src/test/java/com/uid2/admin/v2Router/RouterConfigurationTests.java new file mode 100644 index 00000000..3e358d8d --- /dev/null +++ b/src/test/java/com/uid2/admin/v2Router/RouterConfigurationTests.java @@ -0,0 +1,41 @@ +package com.uid2.admin.v2Router; + +import com.google.inject.CreationException; +import com.google.inject.Guice; +import com.uid2.admin.GuiceMockInjectingModule; +import com.uid2.admin.RequireInjectAnnotationsModule; +import com.uid2.admin.vertx.api.cstg.GetClientSideKeypairsBySite; +import com.uid2.admin.vertx.api.V2RouterModule; +import com.uid2.admin.vertx.service.ClientSideKeypairService; +import com.uid2.shared.middleware.AuthMiddleware; +import lombok.val; +import org.junit.jupiter.api.Test; + +import java.io.InvalidClassException; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +public class RouterConfigurationTests { + @Test + public void IfNeededDependencyIsNotProvided_CreateThrowsAnException() { + assertThrows(CreationException.class, () -> { + val injector = Guice.createInjector(new RequireInjectAnnotationsModule(), new V2RouterModule()); + injector.getInstance(GetClientSideKeypairsBySite.class); + }); + } + + @Test + public void IfNeededDependenciesAreAvailable_ARouterModuleCanBeCreated() throws InvalidClassException { + val keypairServiceMock = mock(ClientSideKeypairService.class); + val authMock = mock(AuthMiddleware.class); + + val injector = Guice.createInjector( + new RequireInjectAnnotationsModule(), + new V2RouterModule(), + new GuiceMockInjectingModule(keypairServiceMock, authMock) + ); + val siteIdRouter = injector.getInstance(GetClientSideKeypairsBySite.class); + assertNotNull(siteIdRouter); + } +} diff --git a/src/test/java/com/uid2/admin/v2Router/SiteId_ClientSideKeypair_Tests.java b/src/test/java/com/uid2/admin/v2Router/SiteId_ClientSideKeypair_Tests.java new file mode 100644 index 00000000..833d305a --- /dev/null +++ b/src/test/java/com/uid2/admin/v2Router/SiteId_ClientSideKeypair_Tests.java @@ -0,0 +1,67 @@ +package com.uid2.admin.v2Router; + +import com.google.inject.Guice; +import com.uid2.admin.GuiceMockInjectingModule; +import com.uid2.admin.RequireInjectAnnotationsModule; +import com.uid2.admin.vertx.api.cstg.ClientSideKeypairResponse; +import com.uid2.admin.vertx.api.cstg.GetClientSideKeypairsBySite; +import com.uid2.admin.vertx.api.V2RouterModule; +import com.uid2.admin.vertx.service.ClientSideKeypairService; +import com.uid2.shared.middleware.AuthMiddleware; +import com.uid2.shared.model.ClientSideKeypair; +import io.vertx.ext.web.RoutingContext; +import lombok.val; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.io.InvalidClassException; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +public class SiteId_ClientSideKeypair_Tests { + @Captor + private ArgumentCaptor keypairResponseCaptor; + @Mock + private ClientSideKeypairService clientSideKeypairMock; + @Mock + private RoutingContext contextMock; + ClientSideKeypair createKeypairMock(int siteId, String publicKey) { + val kpMock = mock(ClientSideKeypair.class); + when(kpMock.getSiteId()).thenReturn(siteId); + when(kpMock.encodePublicKeyToString()).thenReturn(publicKey); + return kpMock; + } + @BeforeEach + void setup() { + MockitoAnnotations.openMocks(this); + } + + @Test + public void WhenTheSiteHasASingleKey_ItIsReturned() throws InvalidClassException { + val siteIdToTest = 5; + val fakePublicKey = "Fake public key"; + val expectedResult = new ArrayList(List.of( + createKeypairMock(siteIdToTest, fakePublicKey) + )); + + when(clientSideKeypairMock.getKeypairsBySite(5)) + .thenReturn(expectedResult); + + val service = new GetClientSideKeypairsBySite(clientSideKeypairMock); + service.handleGetClientSideKeys(contextMock, siteIdToTest); + + verify(contextMock).json(keypairResponseCaptor.capture()); + val response = keypairResponseCaptor.getValue(); + assertEquals(1, response.length); + val item = response[0]; + assertEquals(siteIdToTest, item.siteId); + assertEquals(fakePublicKey, item.publicKey); + } +}