diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index b1d3711bd309..ef00a1e3087d 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -23,9 +23,7 @@ }, "com.yahoo.component.chain.ChainedComponent" : { "superClass" : "com.yahoo.component.AbstractComponent", - "interfaces" : [ - "com.yahoo.component.chain.model.Chainable" - ], + "interfaces" : [ ], "attributes" : [ "public", "abstract" @@ -34,7 +32,9 @@ "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()" + "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)" ], "fields" : [ ] }, @@ -2356,8 +2356,7 @@ "superClass" : "java.lang.Object", "interfaces" : [ "com.yahoo.jdisc.SharedResource", - "com.yahoo.jdisc.http.filter.RequestFilterBase", - "com.yahoo.component.chain.model.Chainable" + "com.yahoo.jdisc.http.filter.RequestFilterBase" ], "attributes" : [ "public", @@ -2401,8 +2400,7 @@ "superClass" : "java.lang.Object", "interfaces" : [ "com.yahoo.jdisc.SharedResource", - "com.yahoo.jdisc.http.filter.ResponseFilterBase", - "com.yahoo.component.chain.model.Chainable" + "com.yahoo.jdisc.http.filter.ResponseFilterBase" ], "attributes" : [ "public", @@ -2428,8 +2426,7 @@ "com.yahoo.jdisc.http.filter.SecurityRequestFilter" : { "superClass" : "java.lang.Object", "interfaces" : [ - "com.yahoo.jdisc.http.filter.RequestFilterBase", - "com.yahoo.component.chain.model.Chainable" + "com.yahoo.jdisc.http.filter.RequestFilterBase" ], "attributes" : [ "public", @@ -2462,8 +2459,7 @@ "com.yahoo.jdisc.http.filter.SecurityResponseFilter" : { "superClass" : "java.lang.Object", "interfaces" : [ - "com.yahoo.jdisc.http.filter.ResponseFilterBase", - "com.yahoo.component.chain.model.Chainable" + "com.yahoo.jdisc.http.filter.ResponseFilterBase" ], "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 109c82fe1c7a..2c2ee5226a67 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,7 +7,6 @@ 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; @@ -20,32 +19,80 @@ * * @author Tony Vaagenes */ -public abstract class ChainedComponent extends AbstractComponent implements Chainable { +public abstract class ChainedComponent extends AbstractComponent { - /** - * The immutable set of dependencies of this. NOTE: the default is only for unit testing. - */ - private Dependencies dependencies = getAnnotatedDependencies(); + /** The immutable set of dependencies of this. NOTE: the default is only for unit testing. */ + private Dependencies dependencies = getDefaultAnnotatedDependencies(); 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(getAnnotatedDependencies()); + 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); } /** - * Returns the configured and declared dependencies of this chainedcomponent + * @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. */ - public Dependencies getDependencies() { return dependencies; } + 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); + } + } -} \ 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 b80746f8b563..c0bc01be27c5 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,7 +148,14 @@ private boolean popAllPhase(OrderedReadyNodes readyNodes) { } private NameProvider getNameProvider(String name) { - return nameProviders.computeIfAbsent(name, ComponentNameProvider::new); + NameProvider nameProvider = nameProviders.get(name); + if (nameProvider != null) + return nameProvider; + else { + nameProvider = new ComponentNameProvider(name); + nameProviders.put(name, nameProvider); + return nameProvider; + } } 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 deleted file mode 100644 index 67ed40f8e77c..000000000000 --- a/container-core/src/main/java/com/yahoo/component/chain/model/Chainable.java +++ /dev/null @@ -1,42 +0,0 @@ -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 7d9aea8fd631..10f8400fa37a 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,8 +7,6 @@ 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; @@ -67,18 +65,16 @@ 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)); @@ -150,10 +146,9 @@ private static void checkFilterTypesCompatible(Chain chain) { } } - @SafeVarargs - private static ComponentRegistry allFiltersWrapped(ComponentRegistry... registries) { + 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))); } @@ -181,21 +176,16 @@ private static boolean isSecurityFilter(Object filter) { } private static class FilterWrapper extends ChainedComponent { - public final Chainable filter; - public final Class filterType; + public final Object filter; + public final Class filterType; - public FilterWrapper(ComponentId id, Chainable filter) { + public FilterWrapper(ComponentId id, Object filter) { super(id); this.filter = filter; this.filterType = getFilterType(filter); } - @Override - public Dependencies getAnnotatedDependencies() { - return filter == null ? super.getAnnotatedDependencies() : filter.getAnnotatedDependencies(); - } - - private static Class getFilterType(Object filter) { + 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 2a38682b080c..49fe5a3af4e9 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,11 +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; -import com.yahoo.component.chain.model.Chainable; - /** * @author gjoranv * @since 2.4 */ -public interface RequestFilterBase extends Chainable { +public interface RequestFilterBase { } 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 e219132a147e..64d54d305d44 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 acbf65920464..6cf006728083 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,11 +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; -import com.yahoo.component.chain.model.Chainable; - /** * @author gjoranv * @since 2.4 */ -public interface ResponseFilterBase extends Chainable { +public interface ResponseFilterBase { } 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 dd2a1719a3ac..cc6633f633a1 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,13 +1,12 @@ // 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, Chainable { +public interface SecurityRequestFilter extends RequestFilterBase { 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 4abceed6d161..05ca9c04d269 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,9 +1,7 @@ // 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; - -public interface SecurityResponseFilter extends ResponseFilterBase, Chainable { +public interface SecurityResponseFilter extends ResponseFilterBase { 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 44a7578fd9f1..7acbaa73fe4f 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,29 +2,15 @@ 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; @@ -63,71 +49,6 @@ 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 7080dce432e1..90e5eeb99ba2 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,7 +2,6 @@ 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; @@ -26,7 +25,7 @@ * * @author bjorncs */ -public abstract class JsonSecurityRequestFilterBase extends ChainedComponent implements SecurityRequestFilter { +public abstract class JsonSecurityRequestFilterBase extends AbstractComponent implements SecurityRequestFilter { private static final Logger log = Logger.getLogger(JsonSecurityRequestFilterBase.class.getName());