diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index ef00a1e3087..a7e555a94aa 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -23,7 +23,9 @@ }, "com.yahoo.component.chain.ChainedComponent" : { "superClass" : "com.yahoo.component.AbstractComponent", - "interfaces" : [ ], + "interfaces" : [ + "com.yahoo.component.chain.model.Chainable" + ], "attributes" : [ "public", "abstract" @@ -32,9 +34,7 @@ "public void (com.yahoo.component.ComponentId)", "protected void ()", "public void initDependencies(com.yahoo.component.chain.dependencies.Dependencies)", - "public com.yahoo.component.chain.dependencies.Dependencies getDependencies()", - "protected com.yahoo.component.chain.dependencies.Dependencies getDefaultAnnotatedDependencies()", - "protected com.yahoo.component.chain.dependencies.Dependencies getAnnotatedDependencies(java.lang.Class, java.lang.Class, java.lang.Class)" + "public com.yahoo.component.chain.dependencies.Dependencies getDependencies()" ], "fields" : [ ] }, @@ -2370,7 +2370,9 @@ }, "com.yahoo.jdisc.http.filter.RequestFilterBase" : { "superClass" : "java.lang.Object", - "interfaces" : [ ], + "interfaces" : [ + "com.yahoo.component.chain.model.Chainable" + ], "attributes" : [ "public", "interface", @@ -2414,7 +2416,9 @@ }, "com.yahoo.jdisc.http.filter.ResponseFilterBase" : { "superClass" : "java.lang.Object", - "interfaces" : [ ], + "interfaces" : [ + "com.yahoo.component.chain.model.Chainable" + ], "attributes" : [ "public", "interface", @@ -2426,7 +2430,8 @@ "com.yahoo.jdisc.http.filter.SecurityRequestFilter" : { "superClass" : "java.lang.Object", "interfaces" : [ - "com.yahoo.jdisc.http.filter.RequestFilterBase" + "com.yahoo.jdisc.http.filter.RequestFilterBase", + "com.yahoo.component.chain.model.Chainable" ], "attributes" : [ "public", @@ -2459,7 +2464,8 @@ "com.yahoo.jdisc.http.filter.SecurityResponseFilter" : { "superClass" : "java.lang.Object", "interfaces" : [ - "com.yahoo.jdisc.http.filter.ResponseFilterBase" + "com.yahoo.jdisc.http.filter.ResponseFilterBase", + "com.yahoo.component.chain.model.Chainable" ], "attributes" : [ "public", diff --git a/container-core/src/main/java/com/yahoo/component/chain/ChainedComponent.java b/container-core/src/main/java/com/yahoo/component/chain/ChainedComponent.java index 2c2ee5226a6..109c82fe1c7 100644 --- a/container-core/src/main/java/com/yahoo/component/chain/ChainedComponent.java +++ b/container-core/src/main/java/com/yahoo/component/chain/ChainedComponent.java @@ -7,6 +7,7 @@ import com.yahoo.component.chain.dependencies.Before; import com.yahoo.component.chain.dependencies.Dependencies; import com.yahoo.component.chain.dependencies.Provides; +import com.yahoo.component.chain.model.Chainable; import java.lang.annotation.Annotation; import java.lang.reflect.InvocationTargetException; @@ -19,80 +20,32 @@ * * @author Tony Vaagenes */ -public abstract class ChainedComponent extends AbstractComponent { +public abstract class ChainedComponent extends AbstractComponent implements Chainable { - /** The immutable set of dependencies of this. NOTE: the default is only for unit testing. */ - private Dependencies dependencies = getDefaultAnnotatedDependencies(); + /** + * The immutable set of dependencies of this. NOTE: the default is only for unit testing. + */ + private Dependencies dependencies = getAnnotatedDependencies(); public ChainedComponent(ComponentId id) { super(id); } - protected ChainedComponent() {} + protected ChainedComponent() { } /** * Called by the container to assign the full set of dependencies to this class (configured and declared). * This is called once before this is started. - * @param dependencies The configured dependencies, that this method will merge with annotated dependencies. + * + * @param dependencies The configured dependencies, that this method will merge with annotated dependencies. */ public void initDependencies(Dependencies dependencies) { - this.dependencies = dependencies.union(getDefaultAnnotatedDependencies()); - } - - /** Returns the configured and declared dependencies of this chainedcomponent */ - public Dependencies getDependencies() { return dependencies; } - - /** This method is here only for legacy reasons, do not override. */ - protected Dependencies getDefaultAnnotatedDependencies() { - Dependencies dependencies = getAnnotatedDependencies(com.yahoo.yolean.chain.Provides.class, com.yahoo.yolean.chain.Before.class, com.yahoo.yolean.chain.After.class); - Dependencies legacyDependencies = getAnnotatedDependencies(Provides.class, Before.class, After.class); - - return dependencies.union(legacyDependencies); + this.dependencies = dependencies.union(getAnnotatedDependencies()); } /** - * @param providesClass The annotation class representing 'provides'. - * @param beforeClass The annotation class representing 'before'. - * @param afterClass The annotation class representing 'after'. - * @return a new {@link Dependencies} created from the annotations given in this component's class. + * Returns the configured and declared dependencies of this chainedcomponent */ - protected Dependencies getAnnotatedDependencies(Class providesClass, - Class beforeClass, - Class afterClass) { - return new Dependencies( - allOf(getSymbols(this, providesClass), this.getClass().getSimpleName(), this.getClass().getName()), - getSymbols(this, beforeClass), - getSymbols(this, afterClass)); - } - - // TODO: move to vespajlib. - private static List allOf(List symbols, String... otherSymbols) { - List result = new ArrayList<>(symbols); - result.addAll(List.of(otherSymbols)); - return result; - } - - - private static List getSymbols(ChainedComponent component, Class annotationClass) { - List result = new ArrayList<>(); - - result.addAll(annotationSymbols(component, annotationClass)); - return result; - } - - private static Collection annotationSymbols(ChainedComponent component, Class annotationClass) { - - try { - Annotation annotation = component.getClass().getAnnotation(annotationClass); - if (annotation != null) { - Object values = annotationClass.getMethod("value").invoke(annotation); - return List.of((String[])values); - } - return List.of(); - - } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) { - throw new RuntimeException(e); - } - } + public Dependencies getDependencies() { return dependencies; } -} +} \ No newline at end of file diff --git a/container-core/src/main/java/com/yahoo/component/chain/dependencies/ordering/ChainBuilder.java b/container-core/src/main/java/com/yahoo/component/chain/dependencies/ordering/ChainBuilder.java index c0bc01be27c..b80746f8b56 100644 --- a/container-core/src/main/java/com/yahoo/component/chain/dependencies/ordering/ChainBuilder.java +++ b/container-core/src/main/java/com/yahoo/component/chain/dependencies/ordering/ChainBuilder.java @@ -148,14 +148,7 @@ private boolean popAllPhase(OrderedReadyNodes readyNodes) { } private NameProvider getNameProvider(String name) { - NameProvider nameProvider = nameProviders.get(name); - if (nameProvider != null) - return nameProvider; - else { - nameProvider = new ComponentNameProvider(name); - nameProviders.put(name, nameProvider); - return nameProvider; - } + return nameProviders.computeIfAbsent(name, ComponentNameProvider::new); } private OrderedReadyNodes getReadyNodes() { diff --git a/container-core/src/main/java/com/yahoo/component/chain/model/Chainable.java b/container-core/src/main/java/com/yahoo/component/chain/model/Chainable.java new file mode 100644 index 00000000000..67ed40f8e77 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/component/chain/model/Chainable.java @@ -0,0 +1,42 @@ +package com.yahoo.component.chain.model; + +import com.yahoo.component.chain.dependencies.After; +import com.yahoo.component.chain.dependencies.Before; +import com.yahoo.component.chain.dependencies.Dependencies; +import com.yahoo.component.chain.dependencies.Provides; + +import java.lang.annotation.Annotation; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +/** + * Components which can be chained together, and where dependency information is provided through annotations. + * + * @author jonmv + */ +public interface Chainable { + + default Dependencies getAnnotatedDependencies() { + Set provides = new LinkedHashSet<>(); + Set before = new LinkedHashSet<>(); + Set after = new LinkedHashSet<>(); + + for (Annotation annotation : getClass().getAnnotations()) { + if (annotation instanceof Provides p) provides.addAll(List.of(p.value())); + if (annotation instanceof com.yahoo.yolean.chain.Provides p) provides.addAll(List.of(p.value())); + + if (annotation instanceof Before b) before.addAll(List.of(b.value())); + if (annotation instanceof com.yahoo.yolean.chain.Before b) before.addAll(List.of(b.value())); + + if (annotation instanceof After a) after.addAll(List.of(a.value())); + if (annotation instanceof com.yahoo.yolean.chain.After a) after.addAll(List.of(a.value())); + } + + provides.add(getClass().getSimpleName()); + provides.add(getClass().getName()); + + return new Dependencies(provides, before, after); + } + +} \ No newline at end of file diff --git a/container-core/src/main/java/com/yahoo/container/http/filter/FilterChainRepository.java b/container-core/src/main/java/com/yahoo/container/http/filter/FilterChainRepository.java index 10f8400fa37..7d9aea8fd63 100644 --- a/container-core/src/main/java/com/yahoo/container/http/filter/FilterChainRepository.java +++ b/container-core/src/main/java/com/yahoo/container/http/filter/FilterChainRepository.java @@ -7,6 +7,8 @@ import com.yahoo.component.chain.Chain; import com.yahoo.component.chain.ChainedComponent; import com.yahoo.component.chain.ChainsConfigurer; +import com.yahoo.component.chain.dependencies.Dependencies; +import com.yahoo.component.chain.model.Chainable; import com.yahoo.component.chain.model.ChainsModel; import com.yahoo.component.chain.model.ChainsModelBuilder; import com.yahoo.component.provider.ComponentRegistry; @@ -65,16 +67,18 @@ private static void addAllFilters(ComponentRegistry destination, } } + @SafeVarargs private static void addAllChains(ComponentRegistry destination, ChainsConfig chainsConfig, - ComponentRegistry... filters) { + ComponentRegistry... filters) { ChainRegistry chainRegistry = buildChainRegistry(chainsConfig, filters); chainRegistry.allComponents() .forEach(chain -> destination.register(chain.getId(), toJDiscChain(chain))); } + @SafeVarargs private static ChainRegistry buildChainRegistry(ChainsConfig chainsConfig, - ComponentRegistry... filters) { + ComponentRegistry... filters) { ChainRegistry chainRegistry = new ChainRegistry<>(); ChainsModel chainsModel = ChainsModelBuilder.buildFromConfig(chainsConfig); ChainsConfigurer.prepareChainRegistry(chainRegistry, chainsModel, allFiltersWrapped(filters)); @@ -146,9 +150,10 @@ private static void checkFilterTypesCompatible(Chain chain) { } } - private static ComponentRegistry allFiltersWrapped(ComponentRegistry... registries) { + @SafeVarargs + private static ComponentRegistry allFiltersWrapped(ComponentRegistry... registries) { ComponentRegistry wrappedFilters = new ComponentRegistry<>(); - for (ComponentRegistry registry : registries) { + for (ComponentRegistry registry : registries) { registry.allComponentsById() .forEach((id, filter) -> wrappedFilters.register(id, new FilterWrapper(id, filter))); } @@ -176,16 +181,21 @@ private static boolean isSecurityFilter(Object filter) { } private static class FilterWrapper extends ChainedComponent { - public final Object filter; - public final Class filterType; + public final Chainable filter; + public final Class filterType; - public FilterWrapper(ComponentId id, Object filter) { + public FilterWrapper(ComponentId id, Chainable filter) { super(id); this.filter = filter; this.filterType = getFilterType(filter); } - private static Class getFilterType(Object filter) { + @Override + public Dependencies getAnnotatedDependencies() { + return filter == null ? super.getAnnotatedDependencies() : filter.getAnnotatedDependencies(); + } + + private static Class getFilterType(Object filter) { if (filter instanceof RequestFilter) return RequestFilter.class; else if (filter instanceof ResponseFilter) diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/RequestFilterBase.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/RequestFilterBase.java index 49fe5a3af4e..2a38682b080 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/RequestFilterBase.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/RequestFilterBase.java @@ -1,9 +1,11 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter; +import com.yahoo.component.chain.model.Chainable; + /** * @author gjoranv * @since 2.4 */ -public interface RequestFilterBase { +public interface RequestFilterBase extends Chainable { } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilter.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilter.java index 64d54d305d4..e219132a147 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilter.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilter.java @@ -6,7 +6,7 @@ import com.yahoo.jdisc.SharedResource; /** - * @author Einar M R Rosenvinge + * @author Einar M R Rosenvinge */ public interface ResponseFilter extends SharedResource, ResponseFilterBase { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilterBase.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilterBase.java index 6cf00672808..acbf6592046 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilterBase.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ResponseFilterBase.java @@ -1,9 +1,11 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter; +import com.yahoo.component.chain.model.Chainable; + /** * @author gjoranv * @since 2.4 */ -public interface ResponseFilterBase { +public interface ResponseFilterBase extends Chainable { } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityRequestFilter.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityRequestFilter.java index cc6633f633a..dd2a1719a3a 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityRequestFilter.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityRequestFilter.java @@ -1,12 +1,13 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter; +import com.yahoo.component.chain.model.Chainable; import com.yahoo.jdisc.handler.ResponseHandler; /** * @author Simon Thoresen Hult */ -public interface SecurityRequestFilter extends RequestFilterBase { +public interface SecurityRequestFilter extends RequestFilterBase, Chainable { void filter(DiscFilterRequest request, ResponseHandler handler); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityResponseFilter.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityResponseFilter.java index 05ca9c04d26..4abceed6d16 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityResponseFilter.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityResponseFilter.java @@ -1,7 +1,9 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter; -public interface SecurityResponseFilter extends ResponseFilterBase { +import com.yahoo.component.chain.model.Chainable; + +public interface SecurityResponseFilter extends ResponseFilterBase, Chainable { void filter(DiscFilterResponse response, RequestView request); diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java index 7acbaa73fe4..44a7578fd9f 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/FilterBindingsProviderTest.java @@ -2,15 +2,29 @@ package com.yahoo.container.jdisc; import com.yahoo.component.ComponentId; +import com.yahoo.component.ComponentSpecification; +import com.yahoo.component.chain.ChainedComponent; +import com.yahoo.component.chain.dependencies.After; +import com.yahoo.component.chain.dependencies.Before; +import com.yahoo.component.chain.dependencies.Provides; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.core.ChainsConfig; +import com.yahoo.container.core.ChainsConfig.Chains; +import com.yahoo.container.core.ChainsConfig.Components; +import com.yahoo.container.core.ChainsConfig.Components.Builder; import com.yahoo.container.http.filter.FilterChainRepository; +import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.RequestFilter; import com.yahoo.jdisc.http.filter.ResponseFilter; +import com.yahoo.jdisc.http.filter.SecurityRequestFilter; +import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain; import com.yahoo.jdisc.http.server.jetty.FilterBindings; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -49,6 +63,71 @@ void requireThatEmptyInputGivesEmptyOutput() { assertTrue(filterBindings.responseFilterIds().isEmpty()); } + @Test + void requireThatChainOrderIsCorrect() { + + List filteredBy = new ArrayList<>(); + + class DummyFilter extends ChainedComponent implements SecurityRequestFilter { + @Override public void filter(DiscFilterRequest request, ResponseHandler handler) { filteredBy.add(this); } + @Override public String toString() { return getClass().getAnnotation(Provides.class).value()[0]; } + } + + @Provides("foo") class Filter1 extends DummyFilter { } + @Provides("bar") @After("foo") class Filter2 extends DummyFilter { } + @Provides("baz") @Before("foo") class Filter3 extends DummyFilter { } + + ComponentRegistry filters = new ComponentRegistry<>(); + + SecurityRequestFilter foo = new Filter1(); + SecurityRequestFilter bar = new Filter2(); + SecurityRequestFilter baz = new Filter3(); + + filters.register(ComponentId.fromString("foo"), foo); + filters.register(ComponentId.fromString("bar"), bar); + filters.register(ComponentId.fromString("baz"), baz); + + ChainsConfig.Builder oneChain = new ChainsConfig.Builder(); + oneChain.components(new Components.Builder().id("foo")) + .components(new Components.Builder().id("bar")) + .components(new Components.Builder().id("baz")) + .chains(new Chains.Builder().id("chain") + .components("bar") + .components("foo") + .components("baz")); + + ((SecurityRequestFilterChain) new FilterChainRepository(oneChain.build(), + new ComponentRegistry<>(), + new ComponentRegistry<>(), + filters, + new ComponentRegistry<>()) + .getFilter(ComponentSpecification.fromString("chain"))).filter((DiscFilterRequest) null, null); + + assertEquals(List.of(baz, foo, bar), filteredBy); + + filteredBy.clear(); + ChainsConfig.Builder childChain = new ChainsConfig.Builder(); + childChain.components(new Builder().id("foo")) + .components(new Builder().id("bar")) + .components(new Builder().id("baz")) + .chains(new Chains.Builder().id("parent") + .components("bar") + .components("baz")) + .chains(new Chains.Builder().id("child") + .components("foo") + .inherits("parent")); + + ((SecurityRequestFilterChain) new FilterChainRepository(childChain.build(), + new ComponentRegistry<>(), + new ComponentRegistry<>(), + filters, + new ComponentRegistry<>()) + .getFilter(ComponentSpecification.fromString("child"))).filter((DiscFilterRequest) null, null); + + assertEquals(List.of(baz, foo, bar), filteredBy); + + } + @Test void requireThatCorrectlyConfiguredFiltersAreIncluded() { final String requestFilter1Id = "requestFilter1"; diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java index 90e5eeb99ba..7080dce432e 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java @@ -2,6 +2,7 @@ package com.yahoo.jdisc.http.filter.security.base; import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.component.chain.ChainedComponent; import com.yahoo.json.Jackson; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; @@ -25,7 +26,7 @@ * * @author bjorncs */ -public abstract class JsonSecurityRequestFilterBase extends AbstractComponent implements SecurityRequestFilter { +public abstract class JsonSecurityRequestFilterBase extends ChainedComponent implements SecurityRequestFilter { private static final Logger log = Logger.getLogger(JsonSecurityRequestFilterBase.class.getName());