Skip to content

Commit

Permalink
Merge pull request #31745 from vespa-engine/revert-31709-jonmv/provid…
Browse files Browse the repository at this point in the history
…e-filter-on-data-plane-filters

Revert "Jonmv/provide filter on data plane filters" MERGEOK
  • Loading branch information
tokle committed Jun 27, 2024
2 parents fbc5944 + 0c4d745 commit 1b83df1
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 179 deletions.
20 changes: 8 additions & 12 deletions container-core/abi-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -34,7 +32,9 @@
"public void <init>(com.yahoo.component.ComponentId)",
"protected void <init>()",
"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" : [ ]
},
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<? extends Annotation> providesClass,
Class<? extends Annotation> beforeClass,
Class<? extends Annotation> 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<String> allOf(List<String> symbols, String... otherSymbols) {
List<String> result = new ArrayList<>(symbols);
result.addAll(List.of(otherSymbols));
return result;
}


private static List<String> getSymbols(ChainedComponent component, Class<? extends Annotation> annotationClass) {
List<String> result = new ArrayList<>();

result.addAll(annotationSymbols(component, annotationClass));
return result;
}

private static Collection<String> annotationSymbols(ChainedComponent component, Class<? extends Annotation> 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);
}
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,18 +65,16 @@ private static void addAllFilters(ComponentRegistry<Object> destination,
}
}

@SafeVarargs
private static void addAllChains(ComponentRegistry<Object> destination,
ChainsConfig chainsConfig,
ComponentRegistry<? extends Chainable>... filters) {
ComponentRegistry<?>... filters) {
ChainRegistry<FilterWrapper> chainRegistry = buildChainRegistry(chainsConfig, filters);
chainRegistry.allComponents()
.forEach(chain -> destination.register(chain.getId(), toJDiscChain(chain)));
}

@SafeVarargs
private static ChainRegistry<FilterWrapper> buildChainRegistry(ChainsConfig chainsConfig,
ComponentRegistry<? extends Chainable>... filters) {
ComponentRegistry<?>... filters) {
ChainRegistry<FilterWrapper> chainRegistry = new ChainRegistry<>();
ChainsModel chainsModel = ChainsModelBuilder.buildFromConfig(chainsConfig);
ChainsConfigurer.prepareChainRegistry(chainRegistry, chainsModel, allFiltersWrapped(filters));
Expand Down Expand Up @@ -150,10 +146,9 @@ private static void checkFilterTypesCompatible(Chain<FilterWrapper> chain) {
}
}

@SafeVarargs
private static ComponentRegistry<FilterWrapper> allFiltersWrapped(ComponentRegistry<? extends Chainable>... registries) {
private static ComponentRegistry<FilterWrapper> allFiltersWrapped(ComponentRegistry<?>... registries) {
ComponentRegistry<FilterWrapper> wrappedFilters = new ComponentRegistry<>();
for (ComponentRegistry<? extends Chainable> registry : registries) {
for (ComponentRegistry<?> registry : registries) {
registry.allComponentsById()
.forEach((id, filter) -> wrappedFilters.register(id, new FilterWrapper(id, filter)));
}
Expand Down Expand Up @@ -181,21 +176,16 @@ private static boolean isSecurityFilter(Object filter) {
}

private static class FilterWrapper extends ChainedComponent {
public final Chainable filter;
public final Class<? extends Chainable> 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<? extends Chainable> getFilterType(Object filter) {
private static Class<?> getFilterType(Object filter) {
if (filter instanceof RequestFilter)
return RequestFilter.class;
else if (filter instanceof ResponseFilter)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import com.yahoo.jdisc.SharedResource;

/**
* @author Einar M R Rosenvinge
* @author <a href="mailto:[email protected]">Einar M R Rosenvinge</a>
*/
public interface ResponseFilter extends SharedResource, ResponseFilterBase {

Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
}
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
Loading

0 comments on commit 1b83df1

Please sign in to comment.