diff --git a/config/spotbugs/suppressions.xml b/config/spotbugs/suppressions.xml index c51071a..116ee98 100644 --- a/config/spotbugs/suppressions.xml +++ b/config/spotbugs/suppressions.xml @@ -8,10 +8,4 @@ - - - - - - \ No newline at end of file diff --git a/metadata/README.md b/metadata/README.md index 95edadd..65ac93f 100644 --- a/metadata/README.md +++ b/metadata/README.md @@ -23,22 +23,19 @@ Authors of Creek based services will build implementations of both aggregate and The inputs of a component define any external resources the component consumes/reads. -All input resources implement the [`ComponentInput`](src/main/java/org/creekservice/api/platform/metadata/ComponentInput.java) -marker interface. +All input resources implement the [`ComponentInput`](src/main/java/org/creekservice/api/platform/metadata/ComponentInput.java) interface. ### Internals The internals of a component define any external resources the component uses internally to perform its function. -All internal resources implement the [`ComponentInternal`](src/main/java/org/creekservice/api/platform/metadata/ComponentInternal.java) -marker interface. +All internal resources implement the [`ComponentInternal`](src/main/java/org/creekservice/api/platform/metadata/ComponentInternal.java) interface. ### Outputs The outputs of a component define any external resources the component produces/writes. -All output resources implement the [`ComponentOutput`](src/main/java/org/creekservice/api/platform/metadata/ComponentOutput.java) -marker interface. +All output resources implement the [`ComponentOutput`](src/main/java/org/creekservice/api/platform/metadata/ComponentOutput.java) interface. ### Service descriptors @@ -55,10 +52,10 @@ to define their public [inputs](#inputs) and [outputs](#outputs). ## Resource Descriptors Resource descriptors define the resources a component uses in its [inputs](#inputs), [internals](#internals) and [outputs](#outputs). -There are corresponding `ComponentInput`, `ComponentInternal` and `ComponentOutput` marker interfaces, respectively. +There are corresponding `ComponentInput`, `ComponentInternal` and `ComponentOutput` interfaces, respectively. -Authors of creek services are not expected to implement the above marker interfaces. Creek extensions, -(such as [creek-kafka][1]), expose their own interfaces, which extend these marker interfaces, and which can be used to +Authors of creek services are not expected to implement the above interfaces. Creek extensions, +(such as [creek-kafka][1]), expose their own interfaces, which extend these interfaces, and which can be used to define a resource the component uses, e.g. a Kafka Topic. > ### NOTE @@ -128,7 +125,7 @@ input, internal or output. ##### Unmanaged Resources -Any resource descriptor that does not implement one of the resource initialization marker interfaces are deemed not +Any resource descriptor that does not implement one of the resource initialization interfaces are deemed not to be initialized by Creek. Such resources must be initialized some other way. #### Resource deployment diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentDescriptor.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentDescriptor.java index 12e67f8..57f288a 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentDescriptor.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentDescriptor.java @@ -36,7 +36,7 @@ *
  • both of the above * */ -public interface ComponentDescriptor { +public interface ComponentDescriptor extends ResourceCollection { /** * @return the unique name of the component within the platform. Can not be {@code null}, blank diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInput.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInput.java index 53c04bd..f304c3b 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInput.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInput.java @@ -16,5 +16,5 @@ package org.creekservice.api.platform.metadata; -/** Marker interface of component inputs. */ +/** Base type for component inputs. */ public interface ComponentInput extends ResourceDescriptor {} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInternal.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInternal.java index 635cfd9..bf1de27 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInternal.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentInternal.java @@ -16,5 +16,5 @@ package org.creekservice.api.platform.metadata; -/** Marker interface of component internals. */ +/** Base type for component internals. */ public interface ComponentInternal extends ResourceDescriptor {} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentOutput.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentOutput.java index e47902e..fb31e1b 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentOutput.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/ComponentOutput.java @@ -16,5 +16,5 @@ package org.creekservice.api.platform.metadata; -/** Marker interface of component outputs. */ +/** Base type for component outputs. */ public interface ComponentOutput extends ResourceDescriptor {} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/CreatableResource.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/CreatableResource.java index 78659ed..837662b 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/CreatableResource.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/CreatableResource.java @@ -16,5 +16,10 @@ package org.creekservice.api.platform.metadata; -/** Marker interface to indicate a resource is {@link ResourceDescriptor#isCreatable creatable}. */ -interface CreatableResource {} +/** + * Base type for creatable resources. + * + *

    Extensions should not implement this directly. Instead, implement one of this interface's + * subtypes, e.g. {@link OwnedResource} or {@link SharedResource}. + */ +public interface CreatableResource extends ResourceDescriptor {} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/OwnedResource.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/OwnedResource.java index 116a9a5..ea3b8e5 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/OwnedResource.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/OwnedResource.java @@ -17,7 +17,7 @@ package org.creekservice.api.platform.metadata; /** - * Marker interface of an owned resources. + * Base type for an owned resources. * *

    A resource can conceptually be either: * @@ -30,7 +30,7 @@ * *

    Resources should inherit at most one of the above interfaces. * - *

    For more information on resource initialization, see - * https://github.com/creek-service/creek-platform/tree/main/metadata#resource-initialization. + *

    For more information on resource initialization, see resource-initialization. */ public interface OwnedResource extends CreatableResource {} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceCollection.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceCollection.java new file mode 100644 index 0000000..e03ce59 --- /dev/null +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceCollection.java @@ -0,0 +1,75 @@ +/* + * Copyright 2023 Creek Contributors (https://github.com/creek-service) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.creekservice.api.platform.metadata; + +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Stream; + +/** A collection of resources. */ +public interface ResourceCollection { + + /** + * The stream of the resources in the collection. + * + *

    In general, callers should call {@link #collectResources(ResourceCollection)}, rather than + * directly calling this method. + * + * @return the resources in the collection. + */ + Stream resources(); + + /** + * Utility method to collect all resources from a collection, including any resources referenced + * by any resources encountered. + * + *

    Child resources are returned earlier in the stream than parents. + * + * @param collection the resource collection to retrieve all resources from. + * @return complete stream of resources. + */ + static Stream collectResources(final ResourceCollection collection) { + final Set visited = new HashSet<>(); + visited.add(collection); + return collection.resources().flatMap(child -> collectResources(child, visited)); + } + + /** + * Utility method to collect all resources referenced by a {@code ResourceDescriptor}, including + * any resources referenced by any resources encountered. + * + *

    Child resources are returned earlier in the stream than parents. + * + *

    In general, callers should call {@link #collectResources(ResourceCollection)}, rather than + * directly calling this method. + * + * @param resource the resource descriptor to retrieve all resources from. + * @param visited the set of resource containers already visited. + * @return complete stream of resources. + */ + static Stream collectResources( + final ResourceDescriptor resource, final Set visited) { + if (!visited.add(resource)) { + // Already processed: + return Stream.empty(); + } + + return Stream.concat( + resource.resources().flatMap(child -> collectResources(child, visited)), + Stream.of(resource)); + } +} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceDescriptor.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceDescriptor.java index eb3d49e..5bec202 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceDescriptor.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/ResourceDescriptor.java @@ -17,9 +17,10 @@ package org.creekservice.api.platform.metadata; import java.net.URI; +import java.util.stream.Stream; -/** Marker interface of resource descriptors. */ -public interface ResourceDescriptor { +/** Base type for describing resource. */ +public interface ResourceDescriptor extends ResourceCollection { /** * A unique identifier for this resource. @@ -42,50 +43,19 @@ public interface ResourceDescriptor { URI id(); /** - * Determine if a resource descriptor is creatable. + * Additional resources this resource requires. * - * @param r the resource descriptor to check. - * @return {@code true} if creatable, {@code false} otherwise. + * @return stream of resources this resource depends on. */ - static boolean isCreatable(final ResourceDescriptor r) { - return r instanceof CreatableResource; - } - - /** - * Determine if a resource descriptor is marked owned. - * - * @param r the resource descriptor to check. - * @return {@code true} if creatable, {@code false} otherwise. - */ - static boolean isOwned(final ResourceDescriptor r) { - return r instanceof OwnedResource; - } - - /** - * Determine if a resource descriptor is unowned. - * - * @param r the resource descriptor to check. - * @return {@code true} if creatable, {@code false} otherwise. - */ - static boolean isUnowned(final ResourceDescriptor r) { - return r instanceof UnownedResource; - } - - /** - * Determine if a resource descriptor is shared. - * - * @param r the resource descriptor to check. - * @return {@code true} if creatable, {@code false} otherwise. - */ - static boolean isShared(final ResourceDescriptor r) { - return r instanceof SharedResource; + default Stream resources() { + return Stream.of(); } /** * Determine if a resource descriptor is unmanaged. * * @param r the r descriptor to check. - * @return {@code true} if creatable, {@code false} otherwise. + * @return {@code true} if unmanaged, {@code false} otherwise. */ static boolean isUnmanaged(final ResourceDescriptor r) { return !(r instanceof SharedResource diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/SharedResource.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/SharedResource.java index d043bcb..3a51f63 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/SharedResource.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/SharedResource.java @@ -17,7 +17,7 @@ package org.creekservice.api.platform.metadata; /** - * Marker interface of an owned resources. + * Base type for shared resources. * *

    A resource can conceptually be either: * @@ -30,7 +30,7 @@ * *

    Resources should inherit at most one of the above interfaces. * - *

    For more information on resource initialization, see - * https://github.com/creek-service/creek-platform/tree/main/metadata#resource-initialization. + *

    For more information on resource initialization, see resource-initialization. */ public interface SharedResource extends CreatableResource {} diff --git a/metadata/src/main/java/org/creekservice/api/platform/metadata/UnownedResource.java b/metadata/src/main/java/org/creekservice/api/platform/metadata/UnownedResource.java index 2b37371..c27e04a 100644 --- a/metadata/src/main/java/org/creekservice/api/platform/metadata/UnownedResource.java +++ b/metadata/src/main/java/org/creekservice/api/platform/metadata/UnownedResource.java @@ -17,7 +17,7 @@ package org.creekservice.api.platform.metadata; /** - * Marker interface of an unowned resources. + * Base type for unowned resources. * *

    A resource can conceptually be either: * @@ -30,7 +30,7 @@ * *

    Resources should inherit at most one of the above interfaces. * - *

    For more information on resource initialization, see - * https://github.com/creek-service/creek-platform/tree/main/metadata#resource-initialization. + *

    For more information on resource initialization, see resource-initialization */ -public interface UnownedResource {} +public interface UnownedResource extends ResourceDescriptor {} diff --git a/metadata/src/test/java/org/creekservice/api/platform/metadata/ResourceCollectionTest.java b/metadata/src/test/java/org/creekservice/api/platform/metadata/ResourceCollectionTest.java new file mode 100644 index 0000000..6947f43 --- /dev/null +++ b/metadata/src/test/java/org/creekservice/api/platform/metadata/ResourceCollectionTest.java @@ -0,0 +1,113 @@ +/* + * Copyright 2023 Creek Contributors (https://github.com/creek-service) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.creekservice.api.platform.metadata; + +import static java.util.stream.Collectors.toList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class ResourceCollectionTest { + + @Mock(extraInterfaces = {ResourceDescriptor.class}) + private ResourceCollection collection; + + @Mock private ResourceDescriptor res0; + @Mock private ResourceDescriptor res1; + @Mock private ResourceDescriptor res2; + + @Test + void shouldCollectUniqueResources() { + // Given: + when(collection.resources()).thenAnswer(inv -> Stream.of(res0, res1, res0, res1)); + when(res0.resources()).thenAnswer(inv -> Stream.of(res1)); + + // When: + final List result = + ResourceCollection.collectResources(collection).collect(toList()); + + // Then: + assertThat(result, containsInAnyOrder(res0, res1)); + } + + @Test + void shouldIgnoreSelfReferences() { + // Given: + when(collection.resources()).thenAnswer(inv -> Stream.of(res0, res1, collection)); + when(res0.resources()).thenAnswer(inv -> Stream.of(collection)); + + // When: + final List result = + ResourceCollection.collectResources(collection).collect(toList()); + + // Then: + assertThat(result, containsInAnyOrder(res0, res1)); + } + + @Test + void shouldNotFollowCircularReferences() { + // Given: + when(collection.resources()).thenAnswer(inv -> Stream.of(res0)); + when(res0.resources()).thenAnswer(inv -> Stream.of(res1)); + when(res1.resources()).thenAnswer(inv -> Stream.of(res0, collection)); + + // When: + final List result = + ResourceCollection.collectResources(collection).collect(toList()); + + // Then: + assertThat(result, containsInAnyOrder(res0, res1)); + } + + @Test + void shouldNotFollowingSelfCircularReferences() { + // Given: + when(collection.resources()).thenAnswer(inv -> Stream.of(res0, res1, collection)); + when(res0.resources()).thenAnswer(inv -> Stream.of(res0)); + + // When: + final List result = + ResourceCollection.collectResources(collection).collect(toList()); + + // Then: + assertThat(result, containsInAnyOrder(res0, res1)); + } + + @Test + void shouldReturnChildResourcesBeforeParent() { + // Given: + when(collection.resources()).thenAnswer(inv -> Stream.of(res0)); + when(res0.resources()).thenAnswer(inv -> Stream.of(res2, res1)); + when(res1.resources()).thenAnswer(inv -> Stream.of(res2)); + + // When: + final List result = + ResourceCollection.collectResources(collection).collect(toList()); + + // Then: + assertThat(result, is(List.of(res2, res1, res0))); + } +} diff --git a/resource/build.gradle.kts b/resource/build.gradle.kts index c2ab192..b7a3643 100644 --- a/resource/build.gradle.kts +++ b/resource/build.gradle.kts @@ -19,9 +19,12 @@ plugins { } val creekVersion : String by extra +val spotBugsVersion : String by extra dependencies { api(project(":metadata")) + implementation("org.creekservice:creek-base-type:$creekVersion") implementation("org.creekservice:creek-observability-logging:$creekVersion") + implementation("com.github.spotbugs:spotbugs-annotations:$spotBugsVersion") } diff --git a/resource/src/main/java/module-info.java b/resource/src/main/java/module-info.java index 5925ce9..70629aa 100644 --- a/resource/src/main/java/module-info.java +++ b/resource/src/main/java/module-info.java @@ -3,6 +3,7 @@ requires transitive creek.platform.metadata; requires creek.base.type; requires creek.observability.logging; + requires com.github.spotbugs.annotations; exports org.creekservice.api.platform.resource; } 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 3869f1c..7cddc3b 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 @@ -18,13 +18,12 @@ import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.groupingBy; -import static org.creekservice.api.platform.metadata.ResourceDescriptor.isOwned; -import static org.creekservice.api.platform.metadata.ResourceDescriptor.isShared; +import static java.util.stream.Collectors.toList; import static org.creekservice.api.platform.metadata.ResourceDescriptor.isUnmanaged; -import static org.creekservice.api.platform.metadata.ResourceDescriptor.isUnowned; import java.net.URI; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -36,8 +35,12 @@ import org.creekservice.api.observability.logging.structured.StructuredLogger; import org.creekservice.api.observability.logging.structured.StructuredLoggerFactory; import org.creekservice.api.platform.metadata.ComponentDescriptor; +import org.creekservice.api.platform.metadata.CreatableResource; import org.creekservice.api.platform.metadata.OwnedResource; +import org.creekservice.api.platform.metadata.ResourceCollection; import org.creekservice.api.platform.metadata.ResourceDescriptor; +import org.creekservice.api.platform.metadata.SharedResource; +import org.creekservice.api.platform.metadata.UnownedResource; import org.creekservice.internal.platform.resource.ComponentValidator; /** @@ -88,12 +91,10 @@ public interface Callbacks { * * @param type the type of the {@code creatableResources}. * @param creatableResources the resource instances to ensure exists and are initialized. - * Resources passed will be {@link ResourceDescriptor#isCreatable creatable}. * @param the creatable resource descriptor type * @throws RuntimeException on unknown resource type */ - void ensure( - Class type, Collection creatableResources); + void ensure(Class type, Collection creatableResources); } /** @@ -131,7 +132,7 @@ public void init(final Collection components) { ensureResources( groupById( components, - resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isShared), + resGroup -> resGroup.stream().anyMatch(SharedResource.class::isInstance), false)); } @@ -152,7 +153,7 @@ public void service(final Collection components) ensureResources( groupById( components, - resGroup -> resGroup.stream().anyMatch(ResourceDescriptor::isOwned), + resGroup -> resGroup.stream().anyMatch(OwnedResource.class::isInstance), true)); } @@ -163,8 +164,8 @@ public void service(final Collection components) * * @param componentsUnderTest components that are being testing * @param otherComponents other components surrounding those being tested, e.g. upstream and - * downstream components. These components can contain {@link ResourceDescriptor#isCreatable - * creatable} resource descriptors needed to know how to create edge resources. + * downstream components. These components can contain {@link CreatableResource creatable} + * resource descriptors needed to know how to create edge resources. */ public void test( final Collection componentsUnderTest, @@ -183,9 +184,10 @@ public void test( groupById( componentsUnderTest, resGroup -> - resGroup.stream().anyMatch(ResourceDescriptor::isUnowned) + resGroup.stream() + .anyMatch(UnownedResource.class::isInstance) && resGroup.stream() - .noneMatch(ResourceDescriptor::isOwned), + .noneMatch(OwnedResource.class::isInstance), true) .collect(Collectors.toMap(group -> group.get(0).id(), Function.identity())); @@ -198,23 +200,25 @@ public void test( ensureResources(unowned.values().stream()); } - @SuppressWarnings({"unchecked", "rawtypes"}) private void ensureResources(final Stream> resGroups) { resGroups .peek(this::validateResourceGroup) .map(this::creatableDescriptor) - .collect(groupingBy(Object::getClass)) + .collect(groupingBy(Object::getClass, LinkedHashMap::new, toList())) .values() - .forEach( - creatableResources -> - callbacks.ensure( - (Class) creatableResources.get(0).getClass(), - (List) creatableResources)); + .forEach(this::ensure); } - private ResourceDescriptor creatableDescriptor(final List resGroup) { + @SuppressWarnings("unchecked") + private void ensure(final List creatableResources) { + final Class type = (Class) creatableResources.get(0).getClass(); + callbacks.ensure(type, creatableResources); + } + + private CreatableResource creatableDescriptor(final List resGroup) { return resGroup.stream() - .filter(ResourceDescriptor::isCreatable) + .filter(CreatableResource.class::isInstance) + .map(CreatableResource.class::cast) .findAny() .orElseThrow(() -> new UncreatableResourceException(resGroup)); } @@ -225,8 +229,8 @@ private Stream> groupById( final boolean validateNonMatchingResGroups) { final Map> grouped = components.stream() - .flatMap(ComponentDescriptor::resources) - .collect(groupingBy(ResourceDescriptor::id)); + .flatMap(ResourceCollection::collectResources) + .collect(groupingBy(ResourceDescriptor::id, LinkedHashMap::new, toList())); final Map>> partitioned = grouped.values().stream().collect(groupingBy(groupPredicate::test)); @@ -247,9 +251,9 @@ private Stream> groupById( @SuppressWarnings("unchecked") private void validateResourceGroup(final List resourceGroup) { final T first = resourceGroup.get(0); - if (isShared(first)) { + if (first instanceof SharedResource) { // if shared, all should be shared. - if (resourceGroup.stream().anyMatch(r -> !isShared(r))) { + if (resourceGroup.stream().anyMatch(r -> !(r instanceof SharedResource))) { throw new ResourceDescriptorMismatchInitializationException( "shared", resourceGroup); } @@ -261,7 +265,8 @@ private void validateResourceGroup(final List } } else { // if owned/unowned mixes allowed. - if (resourceGroup.stream().anyMatch(r -> !(isOwned(r) || isUnowned(r)))) { + if (resourceGroup.stream() + .anyMatch(r -> !(r instanceof OwnedResource || r instanceof UnownedResource))) { throw new ResourceDescriptorMismatchInitializationException( "owned or unowned", resourceGroup); } @@ -303,7 +308,7 @@ private static final class ResourceDescriptorMismatchInitializationException final String type, final List descriptors) { super( "Resource descriptors for resource are tagged with incompatible resource" - + " initialization marker interfaces. First descriptor is marked as a " + + " initialization interfaces. First descriptor is marked as a " + type + " resource, " + "but at least one subsequent descriptor was not " diff --git a/resource/src/main/java/org/creekservice/internal/platform/resource/ComponentValidator.java b/resource/src/main/java/org/creekservice/internal/platform/resource/ComponentValidator.java index 9624c40..84812c5 100644 --- a/resource/src/main/java/org/creekservice/internal/platform/resource/ComponentValidator.java +++ b/resource/src/main/java/org/creekservice/internal/platform/resource/ComponentValidator.java @@ -16,6 +16,7 @@ package org.creekservice.internal.platform.resource; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; @@ -25,7 +26,9 @@ import org.creekservice.api.base.type.CodeLocation; import org.creekservice.api.platform.metadata.AggregateDescriptor; import org.creekservice.api.platform.metadata.ComponentDescriptor; +import org.creekservice.api.platform.metadata.CreatableResource; import org.creekservice.api.platform.metadata.OwnedResource; +import org.creekservice.api.platform.metadata.ResourceCollection; import org.creekservice.api.platform.metadata.ResourceDescriptor; import org.creekservice.api.platform.metadata.ServiceDescriptor; import org.creekservice.api.platform.metadata.SharedResource; @@ -63,13 +66,14 @@ private void validateComponent(final ComponentDescriptor component) { throw new InvalidDescriptorException( "descriptor is neither aggregate and service descriptor", component); } + + validateComponentResources(component); + if (component instanceof AggregateDescriptor) { validateAggregate((AggregateDescriptor) component); } else { validateService((ServiceDescriptor) component); } - - validateComponentResources(component); } private void validateComponentName(final ComponentDescriptor component) { @@ -82,9 +86,19 @@ private void validateComponentName(final ComponentDescriptor component) { } } + @SuppressFBWarnings( + value = "DCN_NULLPOINTER_EXCEPTION", + justification = "validator implementation") private void validateComponentResources(final ComponentDescriptor component) { validateResourcesMethod(component); - component.resources().forEach(r -> validateResource(r, component)); + + try { + ResourceCollection.collectResources(component) + .forEach(res -> validateResource(res, component)); + } catch (final NullPointerException e) { + throw new InvalidDescriptorException( + "contains null resource or resource stream", component); + } } private void validateAggregate(final AggregateDescriptor component) { @@ -96,8 +110,7 @@ private void validateAggregate(final AggregateDescriptor component) { } final List notOwned = - component - .resources() + ResourceCollection.collectResources(component) .filter(r -> !(r instanceof OwnedResource)) .collect(Collectors.toList()); @@ -132,10 +145,6 @@ private void validateResourcesMethod(final ComponentDescriptor component) { private void validateResource( final ResourceDescriptor resource, final ComponentDescriptor component) { - if (resource == null) { - throw new InvalidDescriptorException("contains null resource", component); - } - if (resource.id() == null) { throw new InvalidDescriptorException( "null resource id, resource_type: " + resource.getClass().getSimpleName(), @@ -144,22 +153,35 @@ private void validateResource( final List initialisation = types(resource.getClass()) - .filter(ComponentValidator::isResourceInitializationMarkerInterface) + .filter(ComponentValidator::isResourceInitializationInterface) .map(Class::getSimpleName) .distinct() .sorted() .collect(Collectors.toList()); if (initialisation.size() > 1) { throw new InvalidDescriptorException( - "resource can implement at-most one ResourceInitialization marker interface," + "resource can implement at-most one resource initialization interface," + " but was: " + initialisation, resource, component); } + + if (resource instanceof CreatableResource + && !(resource instanceof SharedResource) + && !(resource instanceof OwnedResource)) { + throw new InvalidDescriptorException( + "creatable resource must implement either the " + + SharedResource.class.getSimpleName() + + " or the " + + OwnedResource.class.getSimpleName() + + " interface", + resource, + component); + } } - private static boolean isResourceInitializationMarkerInterface(final Class type) { + private static boolean isResourceInitializationInterface(final Class type) { return type == OwnedResource.class || type == UnownedResource.class || type == SharedResource.class; 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 e982f0c..fde4649 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 @@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -42,6 +43,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockSettings; import org.mockito.junit.jupiter.MockitoExtension; @@ -145,9 +147,9 @@ void shouldThrowIfResourceGroupContainsSharedAndNonShared() { e.getMessage(), startsWith( "Resource descriptors for resource are tagged with incompatible resource" - + " initialization marker interfaces. First descriptor is marked as a" - + " shared resource, but at least one subsequent descriptor was not" - + " shared. resource: a://1, descriptors: ")); + + " initialization interfaces. First descriptor is marked as a" + + " shared resource, but at least one subsequent descriptor was not" + + " shared. resource: a://1, descriptors: ")); assertThat(e.getMessage(), containsString("sharedResource1")); assertThat(e.getMessage(), containsString("unownedResource1")); } @@ -169,7 +171,7 @@ void shouldThrowIfResourceGroupContainsUnmanagedAndNonManaged() { e.getMessage(), startsWith( "Resource descriptors for resource are tagged with incompatible resource" - + " initialization marker interfaces. First descriptor is marked as a" + + " initialization interfaces. First descriptor is marked as a" + " unmanaged resource, but at least one subsequent descriptor was not" + " unmanaged. resource: a://1, descriptors: ")); assertThat(e.getMessage(), containsString("unmanagedResource1")); @@ -193,9 +195,9 @@ void shouldThrowIfResourceGroupContainsOwnedAndOther() { e.getMessage(), startsWith( "Resource descriptors for resource are tagged with incompatible resource" - + " initialization marker interfaces. First descriptor is marked as a" - + " owned or unowned resource, but at least one subsequent descriptor" - + " was not owned or unowned. resource: a://1, descriptors: ")); + + " initialization interfaces. First descriptor is marked as a owned or" + + " unowned resource, but at least one subsequent descriptor was not" + + " owned or unowned. resource: a://1, descriptors: ")); assertThat(e.getMessage(), containsString("ownedResource1")); assertThat(e.getMessage(), containsString("sharedResource1")); } @@ -217,9 +219,9 @@ void shouldThrowIfResourceGroupContainsUnownedAndOther() { e.getMessage(), startsWith( "Resource descriptors for resource are tagged with incompatible resource" - + " initialization marker interfaces. First descriptor is marked as a" - + " owned or unowned resource, but at least one subsequent descriptor" - + " was not owned or unowned. resource: a://1, descriptors: ")); + + " initialization interfaces. First descriptor is marked as a owned or" + + " unowned resource, but at least one subsequent descriptor was not" + + " owned or unowned. resource: a://1, descriptors: ")); assertThat(e.getMessage(), containsString("unownedResource1")); assertThat(e.getMessage(), containsString("sharedResource1")); } @@ -514,6 +516,37 @@ void shouldThrowOnInvalidComponentUsingActualValidator() { assertThrows(RuntimeException.class, () -> initializer.init(List.of(component0))); } + @SuppressWarnings({"unchecked", "rawtypes"}) + @Test + void shouldEnsureChildResourcesBeforeParents() { + // Given + final ResourceB child = resourceB(OwnedResource.class); + when(ownedResource1.resources()).thenAnswer(inv -> Stream.of(child)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1)); + + // When: + initializer.service(List.of(component0)); + + // Then: + final InOrder inOrder = inOrder(callbacks); + inOrder.verify(callbacks).ensure((Class) child.getClass(), (List) List.of(child)); + inOrder.verify(callbacks) + .ensure((Class) ownedResource1.getClass(), (List) List.of(ownedResource1)); + } + + @Test + void shouldHandleCircularResourceReferences() { + // Given + when(ownedResource1.resources()).thenAnswer(inv -> Stream.of(unownedResource1)); + when(component0.resources()).thenAnswer(inv -> Stream.of(ownedResource1, unownedResource1)); + + // When: + initializer.service(List.of(component0)); + + // Then: did not throw, and: + verify(callbacks).ensure(any(), any()); + } + private static ResourceA resourceA(final int id, final Class extraInterface) { return resourceA(id, withSettings().extraInterfaces(extraInterface)); } diff --git a/resource/src/test/java/org/creekservice/internal/platform/resource/ComponentValidatorTest.java b/resource/src/test/java/org/creekservice/internal/platform/resource/ComponentValidatorTest.java index 4aa4458..f8627c1 100644 --- a/resource/src/test/java/org/creekservice/internal/platform/resource/ComponentValidatorTest.java +++ b/resource/src/test/java/org/creekservice/internal/platform/resource/ComponentValidatorTest.java @@ -19,6 +19,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.matchesRegex; +import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -33,12 +34,11 @@ import org.creekservice.api.platform.metadata.ComponentDescriptor; import org.creekservice.api.platform.metadata.ComponentInput; import org.creekservice.api.platform.metadata.ComponentInternal; -import org.creekservice.api.platform.metadata.ComponentOutput; +import org.creekservice.api.platform.metadata.CreatableResource; import org.creekservice.api.platform.metadata.OwnedResource; import org.creekservice.api.platform.metadata.ResourceDescriptor; import org.creekservice.api.platform.metadata.ServiceDescriptor; import org.creekservice.api.platform.metadata.SharedResource; -import org.creekservice.api.platform.metadata.UnownedResource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -53,10 +53,10 @@ class ComponentValidatorTest { private static final Pattern CODE_LOCATION = Pattern.compile(".*\\(file:/.*creek-platform-metadata-.*\\.jar\\).*", Pattern.DOTALL); - @Mock(name = "jane") + @Mock(name = "bob-text") private ServiceDescriptor service; - @Mock(name = "jane") + @Mock(name = "jane-text") private AggregateDescriptor aggregate; private final ComponentValidator validator = new ComponentValidator(); @@ -64,41 +64,9 @@ class ComponentValidatorTest { @BeforeEach void setUp() { when(service.name()).thenReturn("bob"); - when(aggregate.name()).thenReturn("bob"); + when(aggregate.name()).thenReturn("jane"); when(service.dockerImage()).thenReturn("image"); - - when(service.inputs()) - .thenReturn( - List.of( - mock( - ComponentInput.class, - withSettings().extraInterfaces(UnownedResource.class)), - mock( - ComponentInput.class, - withSettings().extraInterfaces(OwnedResource.class)), - mock( - ComponentInput.class, - withSettings().extraInterfaces(SharedResource.class)))); - when(service.internals()) - .thenReturn( - List.of( - mock( - ComponentInternal.class, - withSettings().extraInterfaces(UnownedResource.class)), - mock( - ComponentInternal.class, - withSettings().extraInterfaces(OwnedResource.class)), - mock(ComponentInternal.class))); - when(service.outputs()) - .thenReturn( - List.of( - mock( - ComponentOutput.class, - withSettings().extraInterfaces(UnownedResource.class)), - mock( - ComponentOutput.class, - withSettings().extraInterfaces(OwnedResource.class)))); } @Test @@ -111,7 +79,8 @@ void shouldThrowOnNullComponentName() { // Then: assertThat( - e.getMessage(), containsString("name can not be null or blank, component: jane")); + e.getMessage(), + containsString("name can not be null or blank, component: bob-text")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } @@ -125,7 +94,8 @@ void shouldThrowOnBlankComponentName() { // Then: assertThat( - e.getMessage(), containsString("name can not be null or blank, component: jane")); + e.getMessage(), + containsString("name can not be null or blank, component: bob-text")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } @@ -194,13 +164,14 @@ void shouldThrowIfResourcesOverride() { @Test void shouldThrowOnNullResource() { // Given: - when(service.resources()).thenReturn(Stream.of((ResourceDescriptor) null)); + when(service.resources()).thenAnswer(inv -> Stream.of((ResourceDescriptor) null)); // When: final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(service)); // Then: - assertThat(e.getMessage(), containsString("contains null resource, component: bob")); + assertThat(e.getMessage(), containsString("contains null resource or resource stream,")); + assertThat(e.getMessage(), containsString(", component: bob")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } @@ -209,7 +180,7 @@ void shouldThrowOnResourceWithNullId() { // Given: final ComponentInput resource = mock(ComponentInput.class, withSettings().name("unowned")); when(resource.id()).thenReturn(null); - when(service.resources()).thenReturn(Stream.of(resource)); + when(service.resources()).thenAnswer(inv -> Stream.of(resource)); // When: final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(service)); @@ -217,10 +188,55 @@ void shouldThrowOnResourceWithNullId() { // Then: assertThat( e.getMessage(), containsString("null resource id, resource_type: ComponentInput$")); + assertThat(e.getMessage(), not(containsString(", resource:"))); assertThat(e.getMessage(), containsString("component: bob")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } + @Test + void shouldThrowOnIssuesWithNestedResources() { + // Given: + final ComponentInput bad = mock(ComponentInput.class, withSettings().name("bad")); + when(bad.id()).thenReturn(null); + + final ComponentInput indirect = mock(ComponentInput.class, withSettings().name("indirect")); + when(indirect.id()).thenReturn(URI.create("res:indirect")); + when(indirect.resources()).thenAnswer(inv -> Stream.of(bad)); + + final ComponentInput direct = mock(ComponentInput.class, withSettings().name("direct")); + when(direct.id()).thenReturn(URI.create("res:direct")); + when(direct.resources()).thenAnswer(inv -> Stream.of(indirect)); + + when(service.resources()).thenAnswer(inv -> Stream.of(direct)); + + // When: + final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(service)); + + // Then: + assertThat( + e.getMessage(), containsString("null resource id, resource_type: ComponentInput$")); + assertThat(e.getMessage(), containsString(", component: bob ")); + assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); + } + + @Test + void shouldThrowOnIssuesWithResourceReturningNullResourceStream() { + // Given: + final ComponentInput bad = mock(ComponentInput.class, withSettings().name("bad")); + when(bad.id()).thenReturn(URI.create("not-null")); + when(bad.resources()).thenReturn(null); + + when(service.resources()).thenAnswer(inv -> Stream.of(bad)); + + // When: + final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(service)); + + // Then: + assertThat(e.getMessage(), containsString("contains null resource or resource stream,")); + assertThat(e.getMessage(), containsString(", component: bob")); + assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); + } + @Test void shouldThrowIfAggregateExposesInternals() { // Given: @@ -236,36 +252,98 @@ void shouldThrowIfAggregateExposesInternals() { assertThat( e.getMessage(), containsString( - "Aggregate should not expose internal resources. internals: [internal]," - + " component: bob")); - assertThat(e.getMessage(), containsString("component: bob")); + "Aggregate should not expose internal resources. internals: [internal]")); + assertThat(e.getMessage(), containsString(", component: jane")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } @Test void shouldThrowIfAggregateExposesNonOwnedResources() { // Given: - when(aggregate.resources()) - .thenReturn(Stream.of(mock(ComponentInput.class, withSettings().name("unowned")))); + final ComponentInput unowned = mock(ComponentInput.class, withSettings().name("unowned")); + when(unowned.id()).thenReturn(URI.create("not-null")); + + when(aggregate.resources()).thenAnswer(inv -> Stream.of(unowned)); + + // When: + final Exception e = + assertThrows(RuntimeException.class, () -> validator.validate(aggregate)); + + // Then: + assertThat(e.getMessage(), containsString("Aggregate should only expose OwnedResource.")); + assertThat(e.getMessage(), containsString(". not_owned: [unowned]")); + assertThat(e.getMessage(), containsString(", component: jane")); + assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); + } + + @Test + void shouldThrowIfAggregateExposesNestedNonOwnedResources() { + // Given: + final ComponentInput unowned = mock(ComponentInput.class, withSettings().name("unowned")); + when(unowned.id()).thenReturn(URI.create("res:unowned")); + + final ComponentInput indirect = + mock( + ComponentInput.class, + withSettings().extraInterfaces(OwnedResource.class).name("indirect")); + when(indirect.id()).thenReturn(URI.create("res:indirect")); + when(indirect.resources()).thenAnswer(inv -> Stream.of(unowned)); + + final ComponentInput direct = + mock( + ComponentInput.class, + withSettings().extraInterfaces(OwnedResource.class).name("direct")); + when(direct.id()).thenReturn(URI.create("res:direct")); + when(direct.resources()).thenAnswer(inv -> Stream.of(indirect)); + + when(aggregate.resources()).thenAnswer(inv -> Stream.of(direct)); // When: final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(aggregate)); + // Then: + assertThat(e.getMessage(), containsString("Aggregate should only expose OwnedResource.")); + assertThat(e.getMessage(), containsString(". not_owned: [unowned]")); + assertThat(e.getMessage(), containsString(", component: jane")); + assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); + } + + @Test + void shouldThrowIfResourceImplementsMultipleInitializationInterfaces() { + // Given: + final ComponentInput bad = + mock( + ComponentInput.class, + withSettings() + .extraInterfaces(SharedResource.class, OwnedResource.class) + .name("bad")); + when(bad.id()).thenReturn(URI.create("bad:resource")); + when(service.resources()).thenAnswer(inv -> Stream.of(bad)); + + // When: + final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(service)); + // Then: assertThat( e.getMessage(), containsString( - "Aggregate should only expose OwnedResource. not_owned: [unowned]," - + " component: bob")); - assertThat(e.getMessage(), containsString("component: bob")); + "resource can implement at-most one resource initialization interface" + + ", but was: [OwnedResource, SharedResource]")); + assertThat(e.getMessage(), containsString(", resource: bad:resource")); + assertThat(e.getMessage(), containsString(", component: bob")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } @Test - void shouldThrowIfResourceImplementsMultipleInitializationMarkers() { + void shouldThrowIfResourceImplementsCreatableButNotOwnedOrShared() { // Given: - when(service.resources()).thenReturn(Stream.of(new BadResourceDescriptor())); + final ComponentInput bad = + mock( + ComponentInput.class, + withSettings().extraInterfaces(CreatableResource.class).name("bad")); + when(bad.id()).thenReturn(URI.create("bad:resource")); + when(service.resources()).thenAnswer(inv -> Stream.of(bad)); // When: final Exception e = assertThrows(RuntimeException.class, () -> validator.validate(service)); @@ -274,10 +352,10 @@ void shouldThrowIfResourceImplementsMultipleInitializationMarkers() { assertThat( e.getMessage(), containsString( - "resource can implement at-most one ResourceInitialization marker" - + " interface, but was: [OwnedResource, SharedResource], resource:" - + " bad:resource")); - assertThat(e.getMessage(), containsString("component: bob")); + "creatable resource must implement either the SharedResource or the" + + " OwnedResource interface,")); + assertThat(e.getMessage(), containsString(", resource: bad:resource")); + assertThat(e.getMessage(), containsString(", component: bob")); assertThat(e.getMessage(), matchesRegex(CODE_LOCATION)); } @@ -331,6 +409,26 @@ void shouldNotThrowIfEverythingIsOK() { validator.validate(service); } + @Test + void shouldWorkWithCircularResourceReferences() { + // Given: + final ComponentInput indirect = mock(ComponentInput.class, withSettings().name("indirect")); + when(indirect.id()).thenReturn(URI.create("res:indirect")); + + final ComponentInput direct = mock(ComponentInput.class, withSettings().name("direct")); + when(direct.id()).thenReturn(URI.create("res:direct")); + + when(indirect.resources()).thenAnswer(inv -> Stream.of(direct)); + when(direct.resources()).thenAnswer(inv -> Stream.of(indirect)); + + when(service.resources()).thenAnswer(inv -> Stream.of(direct)); + + // When: + validator.validate(service); + + // Then: did not throw + } + private static final class OverridingServiceDescriptor implements ServiceDescriptor { @Override public String name() { @@ -359,13 +457,4 @@ public String dockerImage() { return "image"; } } - - private static final class BadResourceDescriptor - implements ResourceDescriptor, SharedResource, OwnedResource { - - @Override - public URI id() { - return URI.create("bad:resource"); - } - } }