From 00925bfa3204d06f26570314996ddc2ae7789e8f Mon Sep 17 00:00:00 2001 From: Big Andy <8012398+big-andy-coates@users.noreply.github.com> Date: Sat, 23 Dec 2023 13:27:58 +0000 Subject: [PATCH] Validate non-owned resource groups. When initialising `service` or `test` state, we should validate all resource groups associated with a service(s), not just owned resources, as discrepancies cause bugs. --- .../resource/ResourceInitializer.java | 35 +++++--- .../resource/ResourceInitializerTest.java | 90 +++++++++++-------- 2 files changed, 77 insertions(+), 48 deletions(-) diff --git a/resource/src/main/java/org/creekservice/api/platform/resource/ResourceInitializer.java b/resource/src/main/java/org/creekservice/api/platform/resource/ResourceInitializer.java index 7f6aeca..3869f1c 100644 --- a/resource/src/main/java/org/creekservice/api/platform/resource/ResourceInitializer.java +++ b/resource/src/main/java/org/creekservice/api/platform/resource/ResourceInitializer.java @@ -122,6 +122,8 @@ public static ResourceInitializer resourceInitializer(final Callbacks callbacks) * @param components components to search for resources. */ public void init(final Collection components) { + components.forEach(componentValidator::validate); + LOGGER.debug( "Initializing resources", log -> log.with("stage", "init").with("components", componentNames(components))); @@ -129,7 +131,8 @@ public void init(final Collection components) { ensureResources( groupById( components, - resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isShared))); + resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isShared), + false)); } /** @@ -140,6 +143,8 @@ public void init(final Collection components) { * @param components components to search for resources. */ public void service(final Collection components) { + components.forEach(componentValidator::validate); + LOGGER.debug( "Initializing resources", log -> log.with("stage", "service").with("components", componentNames(components))); @@ -147,7 +152,8 @@ public void service(final Collection components) ensureResources( groupById( components, - resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isOwned))); + resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isOwned), + true)); } /** @@ -163,6 +169,8 @@ public void service(final Collection components) public void test( final Collection componentsUnderTest, final Collection otherComponents) { + componentsUnderTest.forEach(componentValidator::validate); + otherComponents.forEach(componentValidator::validate); LOGGER.debug( "Initializing resources", @@ -177,12 +185,14 @@ public void test( resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isUnowned) && resGroup.stream() - .noneMatch(ResourceDescriptor::isOwned)) + .noneMatch(ResourceDescriptor::isOwned), + true) .collect(Collectors.toMap(group -> group.get(0).id(), Function.identity())); groupById( otherComponents, - resGroup -> resGroup.stream().anyMatch(r -> unowned.containsKey(r.id()))) + resGroup -> resGroup.stream().anyMatch(r -> unowned.containsKey(r.id())), + false) .forEach(resGroup -> unowned.get(resGroup.get(0).id()).addAll(resGroup)); ensureResources(unowned.values().stream()); @@ -211,18 +221,21 @@ private ResourceDescriptor creatableDescriptor(final List re private Stream> groupById( final Collection components, - final Predicate> groupPredicate) { + final Predicate> groupPredicate, + final boolean validateNonMatchingResGroups) { final Map> grouped = components.stream() - .flatMap(this::getResources) + .flatMap(ComponentDescriptor::resources) .collect(groupingBy(ResourceDescriptor::id)); - return grouped.values().stream().filter(groupPredicate); - } + final Map>> partitioned = + grouped.values().stream().collect(groupingBy(groupPredicate::test)); + + if (validateNonMatchingResGroups) { + partitioned.getOrDefault(false, List.of()).forEach(this::validateResourceGroup); + } - private Stream getResources(final ComponentDescriptor component) { - componentValidator.validate(component); - return component.resources(); + return partitioned.getOrDefault(true, List.of()).stream(); } /** diff --git a/resource/src/test/java/org/creekservice/api/platform/resource/ResourceInitializerTest.java b/resource/src/test/java/org/creekservice/api/platform/resource/ResourceInitializerTest.java index d5d8c4f..e982f0c 100644 --- a/resource/src/test/java/org/creekservice/api/platform/resource/ResourceInitializerTest.java +++ b/resource/src/test/java/org/creekservice/api/platform/resource/ResourceInitializerTest.java @@ -131,8 +131,8 @@ void shouldThrowIfComponentValidationFails() { @Test void shouldThrowIfResourceGroupContainsSharedAndNonShared() { // Given: - when(component0.resources()).thenReturn(Stream.of(sharedResource1)); - when(component1.resources()).thenReturn(Stream.of(unownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); // When: final Exception e = @@ -155,8 +155,8 @@ void shouldThrowIfResourceGroupContainsSharedAndNonShared() { @Test void shouldThrowIfResourceGroupContainsUnmanagedAndNonManaged() { // Given: - when(component0.resources()).thenReturn(Stream.of(unmanagedResource1)); - when(component1.resources()).thenReturn(Stream.of(sharedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(unmanagedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); // When: final Exception e = @@ -179,8 +179,8 @@ void shouldThrowIfResourceGroupContainsUnmanagedAndNonManaged() { @Test void shouldThrowIfResourceGroupContainsOwnedAndOther() { // Given: - when(component0.resources()).thenReturn(Stream.of(ownedResource1)); - when(component1.resources()).thenReturn(Stream.of(sharedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); // When: final Exception e = @@ -203,8 +203,8 @@ void shouldThrowIfResourceGroupContainsOwnedAndOther() { @Test void shouldThrowIfResourceGroupContainsUnownedAndOther() { // Given: - when(component0.resources()).thenReturn(Stream.of(unownedResource1)); - when(component1.resources()).thenReturn(Stream.of(sharedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); // When: final Exception e = @@ -229,8 +229,8 @@ void shouldThrowIfResourceGroupContainsUnownedAndOther() { void shouldCallbackValidateEachResGroupOnInit() { // Given: final ResourceA sharedResource2 = resourceA(1, SharedResource.class); - when(component0.resources()).thenReturn(Stream.of(sharedResource1)); - when(component1.resources()).thenReturn(Stream.of(sharedResource2)); + when(component0.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(sharedResource2)); // When: initializer.init(List.of(component0, component1)); @@ -246,13 +246,20 @@ void shouldCallbackValidateEachResGroupOnInit() { @Test void shouldCallbackValidateEachResGroupOnService() { // Given: - when(component0.resources()).thenReturn(Stream.of(ownedResource1)); - when(component1.resources()).thenReturn(Stream.of(unownedResource1)); + when(unmanagedResource1.id()).thenReturn(URI.create("a://diff")); + when(component0.resources()) + .thenAnswer(inv -> Stream.of(ownedResource1, unmanagedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); // When: initializer.service(List.of(component0, component1)); // Then: + verify(callbacks) + .validate( + (Class) unmanagedResource1.getClass(), + List.of(unmanagedResource1)); + verify(callbacks) .validate( (Class) ownedResource1.getClass(), @@ -263,13 +270,20 @@ void shouldCallbackValidateEachResGroupOnService() { @Test void shouldCallbackValidateEachResGroupOnTest() { // Given: - when(component0.resources()).thenReturn(Stream.of(unownedResource1)); - when(component1.resources()).thenReturn(Stream.of(ownedResource1)); + when(unmanagedResource1.id()).thenReturn(URI.create("a://diff")); + when(component0.resources()) + .thenAnswer(inv -> Stream.of(unownedResource1, unmanagedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); // When: initializer.test(List.of(component0), List.of(component1)); // Then: + verify(callbacks) + .validate( + (Class) unmanagedResource1.getClass(), + List.of(unmanagedResource1)); + verify(callbacks) .validate( (Class) unownedResource1.getClass(), @@ -279,8 +293,8 @@ void shouldCallbackValidateEachResGroupOnTest() { @Test void shouldThrowIfValidateCallbackThrows() { // Given: - when(component0.resources()).thenReturn(Stream.of(ownedResource1)); - when(component1.resources()).thenReturn(Stream.of(unownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); final RuntimeException expected = new RuntimeException("BIG BADA BOOM"); doThrow(expected).when(callbacks).validate(any(), any()); @@ -297,8 +311,9 @@ void shouldThrowIfValidateCallbackThrows() { @Test void shouldNotInitializeAnyResourceOnInitIfNoSharedResources() { // Given: - when(component0.resources()).thenReturn(Stream.of(ownedResource1, unmanagedResource1)); - when(component1.resources()).thenReturn(Stream.of(unownedResource1)); + when(component0.resources()) + .thenAnswer(inv -> Stream.of(ownedResource1, unmanagedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); // When: initializer.init(List.of(component0, component1)); @@ -312,8 +327,8 @@ void shouldNotInitializeAnyResourcesOnServiceIfNoOwnedResources() { // Given: final ResourceA shared = resourceA(2, SharedResource.class); final ResourceA unowned = resourceA(3, UnownedResource.class); - when(component0.resources()).thenReturn(Stream.of(shared, unmanagedResource1)); - when(component1.resources()).thenReturn(Stream.of(unowned)); + when(component0.resources()).thenAnswer(inv -> Stream.of(shared, unmanagedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(unowned)); // When: initializer.service(List.of(component0, component1)); @@ -328,9 +343,9 @@ void shouldNotInitializeAnyResourcesOnTestIfNoUnownedResources() { final ResourceA shared = resourceA(2, SharedResource.class); final ResourceA unowned = resourceA(3, UnownedResource.class); final ResourceA owned = resourceA(4, OwnedResource.class); - when(component0.resources()).thenReturn(Stream.of(unmanagedResource1, shared)); + when(component0.resources()).thenAnswer(inv -> Stream.of(unmanagedResource1, shared)); when(component1.resources()) - .thenReturn(Stream.of(unmanagedResource1, shared, unowned, owned)); + .thenAnswer(inv -> Stream.of(unmanagedResource1, shared, unowned, owned)); // When: initializer.test(List.of(component0), List.of(component1)); @@ -343,8 +358,8 @@ void shouldNotInitializeAnyResourcesOnTestIfNoUnownedResources() { void shouldNotInitializeAnyResourcesOnTestIfUnownedResourcesHaveOwnedDescriptorInComponentsUnderTest() { // Given: - when(component0.resources()).thenReturn(Stream.of(unownedResource1)); - when(component1.resources()).thenReturn(Stream.of(ownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); // When: initializer.test(List.of(component0, component1), List.of()); @@ -359,8 +374,8 @@ void shouldNotInitializeUnmanagedGroups() { final ResourceDescriptor unmanagedResource1b = mock(ResourceDescriptor.class); when(unmanagedResource1b.id()).thenReturn(A1_ID); - when(component0.resources()).thenReturn(Stream.of(unmanagedResource1)); - when(component1.resources()).thenReturn(Stream.of(unmanagedResource1b)); + when(component0.resources()).thenAnswer(inv -> Stream.of(unmanagedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(unmanagedResource1b)); // When: initializer.init(List.of(component0, component1)); @@ -372,8 +387,9 @@ void shouldNotInitializeUnmanagedGroups() { @Test void shouldThrowOnUncreatableResource() { // Given: - when(component0.resources()).thenReturn(Stream.of(unownedResource1)); - when(component1.resources()).thenReturn(Stream.of()); // Missing owned resource descriptor + when(component0.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); + when(component1.resources()) + .thenAnswer(inv -> Stream.of()); // Missing owned resource descriptor // When: final Exception e = @@ -395,8 +411,8 @@ void shouldThrowOnUncreatableResource() { void shouldEnsureSharedResource() { // Given: final ResourceA sharedResource2 = resourceA(2, SharedResource.class); - when(component0.resources()).thenReturn(Stream.of(sharedResource1)); - when(component1.resources()).thenReturn(Stream.of(sharedResource2, sharedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(sharedResource2, sharedResource1)); // When: initializer.init(List.of(component0, component1)); @@ -413,8 +429,8 @@ void shouldEnsureSharedResource() { void shouldEnsureOwnedResource() { // Given: final ResourceA ownedResource2 = resourceA(2, OwnedResource.class); - when(component0.resources()).thenReturn(Stream.of(ownedResource1)); - when(component1.resources()).thenReturn(Stream.of(ownedResource2, unownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); + when(component1.resources()).thenAnswer(inv -> Stream.of(ownedResource2, unownedResource1)); // When: initializer.service(List.of(component0, component1)); @@ -432,8 +448,8 @@ void shouldEnsureUnownedResource() { // Given: final ResourceA ownedResource2 = resourceA(2, OwnedResource.class); final ResourceA unownedResource2 = resourceA(2, UnownedResource.class); - when(component0.resources()).thenReturn(Stream.of(unownedResource2)); - when(component1.resources()).thenReturn(Stream.of(ownedResource2, unownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(unownedResource2)); + when(component1.resources()).thenAnswer(inv -> Stream.of(ownedResource2, unownedResource1)); // When: initializer.test(List.of(component0), List.of(component1)); @@ -447,7 +463,7 @@ void shouldThrowIfEnsureThrows() { // Given: final RuntimeException expected = new RuntimeException("boom"); doThrow(expected).when(callbacks).ensure(any(), any()); - when(component0.resources()).thenReturn(Stream.of(ownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); // When: final Exception e = @@ -463,7 +479,7 @@ void shouldThrowIfEnsureThrows() { void shouldEnsureGroupingByHandler() { // Given final ResourceB ownedResourceB = resourceB(OwnedResource.class); - when(component0.resources()).thenReturn(Stream.of(ownedResource1, ownedResourceB)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1, ownedResourceB)); // When: initializer.service(List.of(component0)); @@ -478,7 +494,7 @@ void shouldThrowOnUnknownResourceType() { // Given: final NullPointerException expected = new NullPointerException("unknown"); doThrow(expected).when(callbacks).ensure(any(), any()); - when(component0.resources()).thenReturn(Stream.of(sharedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(sharedResource1)); // When: final Exception e =