Skip to content

Commit 9bde5b2

Browse files
authored
Implement kroxylicious#3101 by adding @DeprecatedPluginName (kroxylicious#3111)
* Implement kroxylicious#3101 Signed-off-by: Tom Bentley <tbentley@redhat.com>
1 parent 2bfeb1f commit 9bde5b2

16 files changed

Lines changed: 442 additions & 13 deletions
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright Kroxylicious Authors.
3+
*
4+
* Licensed under the Apache Software License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
7+
package io.kroxylicious.proxy.plugin;
8+
9+
import java.lang.annotation.ElementType;
10+
import java.lang.annotation.Retention;
11+
import java.lang.annotation.RetentionPolicy;
12+
import java.lang.annotation.Target;
13+
14+
/**
15+
* <p>Annotates a {@linkplain Plugin @Plugin}
16+
* implementation class whose
17+
* fully-qualified type name has been changed
18+
* and whose old name should no longer be used
19+
* to refer to it.</p>
20+
*
21+
* <p>Plugin implementations should ideally have a single,
22+
* canonical name (the fully-qualified class name), so
23+
* this annotation is not intended to provide a general purpose
24+
* plugin aliasing facility.
25+
* Instead, it is provided as a way of "renaming a plugin"
26+
* while maintaining backwards compatibility with
27+
* configuration files that continue to use the old
28+
* implementation class name.
29+
* When a plugin implementation is instantiated using the old name
30+
* a warning will be logged prompting the
31+
* end user to update their configuration to use
32+
* the new name.</p>
33+
*
34+
* <p>If a plugin implementation class itself is deprecated then
35+
* the {@linkplain Deprecated @Deprecated}
36+
* annotation should be used on that class instead.</p>
37+
*/
38+
@Retention(RetentionPolicy.RUNTIME)
39+
@Target(ElementType.TYPE)
40+
public @interface DeprecatedPluginName {
41+
/**
42+
* The fully qualified class name by which this plugin was previously known.
43+
*/
44+
String oldName();
45+
46+
/**
47+
* Returns the version in which the name became deprecated.
48+
*/
49+
String since() default "";
50+
51+
}

kroxylicious-runtime/src/main/java/io/kroxylicious/proxy/config/ServiceBasedPluginFactoryRegistry.java

Lines changed: 110 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package io.kroxylicious.proxy.config;
88

99
import java.util.Collections;
10+
import java.util.Comparator;
1011
import java.util.HashMap;
1112
import java.util.HashSet;
1213
import java.util.Map;
@@ -20,9 +21,12 @@
2021
import org.slf4j.Logger;
2122
import org.slf4j.LoggerFactory;
2223

24+
import io.kroxylicious.proxy.plugin.DeprecatedPluginName;
2325
import io.kroxylicious.proxy.plugin.Plugin;
2426
import io.kroxylicious.proxy.plugin.UnknownPluginInstanceException;
2527

28+
import edu.umd.cs.findbugs.annotations.Nullable;
29+
2630
/**
2731
* A {@link PluginFactoryRegistry} that is implemented using {@link ServiceLoader} discovery.
2832
*/
@@ -57,7 +61,9 @@ private static Map<String, ProviderAndConfigType> loadProviders(Class<?> pluginI
5761
}
5862
else {
5963
ProviderAndConfigType providerAndConfigType = new ProviderAndConfigType(provider, annotation.configType());
60-
Stream.of(providerType.getName(), providerType.getSimpleName()).forEach(name2 -> nameToProviders.compute(name2, (k2, v) -> {
64+
Stream<String> names = Stream.of(providerType.getName(), providerType.getSimpleName());
65+
names = maybeAddOldNames(providerType, names);
66+
names.forEach(name -> nameToProviders.compute(name, (k2, v) -> {
6167
if (v == null) {
6268
v = new HashSet<>();
6369
}
@@ -70,19 +76,79 @@ private static Map<String, ProviderAndConfigType> loadProviders(Class<?> pluginI
7076
Collectors.partitioningBy(e -> e.getValue().size() == 1));
7177
if (LOGGER.isWarnEnabled()) {
7278
for (Map.Entry<String, Set<ProviderAndConfigType>> ambiguousInstanceNameToProviders : bySingleton.get(false)) {
73-
LOGGER.warn("'{}' would be an ambiguous reference to a {} provider. "
74-
+ "It could refer to any of {}"
75-
+ " so to avoid ambiguous behaviour those fully qualified names must be used",
76-
ambiguousInstanceNameToProviders.getKey(),
77-
pluginInterface.getSimpleName(),
78-
ambiguousInstanceNameToProviders.getValue().stream().map(p -> p.provider().type().getName()).collect(Collectors.joining(", ")));
79+
String ambiguousKey = ambiguousInstanceNameToProviders.getKey();
80+
var implementationClasses = ambiguousInstanceNameToProviders.getValue().stream()
81+
.map(p -> p.provider().type())
82+
.sorted(Comparator.comparing(Class::getName))
83+
.toList();
84+
var fqCollision = implementationClasses.stream().filter(c -> c.isAnnotationPresent(DeprecatedPluginName.class))
85+
.flatMap(c -> implementationClasses.stream()
86+
.filter(c2 -> {
87+
var cOldName = c.getAnnotation(DeprecatedPluginName.class).oldName();
88+
return !c.equals(c2) && (c2.getName().equals(cOldName)
89+
|| (c2.isAnnotationPresent(DeprecatedPluginName.class) &&
90+
c2.getAnnotation(DeprecatedPluginName.class).oldName().equals(cOldName)));
91+
})
92+
.map(c2 -> Map.entry(c, c2)))
93+
.findFirst();
94+
if (fqCollision.isPresent()) {
95+
var entry = fqCollision.get();
96+
var annotatedClass = entry.getKey();
97+
var classWithCollidingFqName = entry.getValue();
98+
LOGGER.warn("Plugin implementation class {} is annotated with @{}(oldName=\"{}\") which collides with the plugin implementation class {}. "
99+
+ "You must remove one of these classes from the class path.",
100+
annotatedClass.getName(),
101+
DeprecatedPluginName.class.getSimpleName(),
102+
annotatedClass.getAnnotation(DeprecatedPluginName.class).oldName(),
103+
classWithCollidingFqName.getName());
104+
throw new RuntimeException("Ambiguous plugin implementation name '" + ambiguousKey + "'");
105+
}
106+
else {
107+
LOGGER.warn("'{}' would be an ambiguous reference to a {} provider. "
108+
+ "It could refer to any of {}"
109+
+ " so to avoid ambiguous behaviour those fully qualified names must be used",
110+
ambiguousKey,
111+
pluginInterface.getSimpleName(),
112+
implementationClasses.stream()
113+
.map(Class::getName)
114+
.collect(Collectors.joining(", ")));
115+
}
79116
}
80117
}
81118
return bySingleton.get(true).stream().collect(Collectors.toMap(
82119
Map.Entry::getKey,
83120
e -> e.getValue().iterator().next()));
84121
}
85122

123+
private static Stream<String> maybeAddOldNames(Class<?> providerType, Stream<String> names) {
124+
if (providerType.isAnnotationPresent(DeprecatedPluginName.class)) {
125+
String oldName = providerType.getAnnotation(DeprecatedPluginName.class).oldName();
126+
if (oldName.equals(providerType.getName())) {
127+
LOGGER.warn("@DeprecatedPluginName annotation on {} "
128+
+ "specifies an oldName == newName. "
129+
+ "This annotation is not being used correctly.",
130+
providerType.getName());
131+
}
132+
else {
133+
names = Stream.concat(names, Stream.of(oldName));
134+
String shortName = simpleName(oldName);
135+
if (shortName != null) {
136+
names = Stream.concat(names, Stream.of(shortName));
137+
}
138+
}
139+
}
140+
return names;
141+
}
142+
143+
private static @Nullable String simpleName(String oldName) {
144+
String substring = null;
145+
var idx = oldName.lastIndexOf('.');
146+
if (idx != -1 && idx != oldName.length() - 1) {
147+
substring = oldName.substring(idx + 1);
148+
}
149+
return substring;
150+
}
151+
86152
@Override
87153
public <P> PluginFactory<P> pluginFactory(Class<P> pluginClass) {
88154
var nameToProvider = load(pluginClass);
@@ -95,11 +161,8 @@ public P pluginInstance(String instanceName) {
95161
var provider = nameToProvider.get(instanceName);
96162
if (provider != null) {
97163
Class<?> type = provider.provider().type();
98-
if (type.isAnnotationPresent(Deprecated.class)) {
99-
LOGGER.warn("{} plugin with id {} is deprecated",
100-
pluginClass.getName(),
101-
instanceName);
102-
}
164+
maybeWarnAboutDeprecatedPluginClass(instanceName, type, pluginClass);
165+
maybeWarnAboutDeprecatedPluginName(instanceName, type, pluginClass);
103166
return pluginClass.cast(provider.provider().get());
104167
}
105168
throw unknownPluginInstanceException(instanceName);
@@ -126,4 +189,39 @@ public Set<String> registeredInstanceNames() {
126189
}
127190
};
128191
}
192+
193+
private static <P> void maybeWarnAboutDeprecatedPluginName(String instanceName, Class<?> type, Class<P> pluginClass) {
194+
if (type.isAnnotationPresent(DeprecatedPluginName.class)) {
195+
DeprecatedPluginName deprecatedName = type.getAnnotation(DeprecatedPluginName.class);
196+
if (isOldInstanceName(instanceName, deprecatedName, type)) {
197+
LOGGER.warn("{} plugin with name '{}' should now be referred to using the name '{}'. "
198+
+ "The plugin has been renamed and "
199+
+ "in the future the old name '{}' will cease to work.",
200+
pluginClass.getName(),
201+
instanceName,
202+
type.getName(),
203+
instanceName);
204+
}
205+
}
206+
}
207+
208+
private static <P> void maybeWarnAboutDeprecatedPluginClass(String instanceName, Class<?> type, Class<P> pluginClass) {
209+
if (type.isAnnotationPresent(Deprecated.class)) {
210+
LOGGER.warn("{} plugin with name '{}' is deprecated.",
211+
pluginClass.getName(),
212+
instanceName);
213+
}
214+
}
215+
216+
private static boolean isOldInstanceName(String instanceName, DeprecatedPluginName deprecatedName, Class<?> type) {
217+
return instanceName.equals(deprecatedName.oldName())
218+
|| (!isFqName(instanceName) // is a short name
219+
&& !instanceName.equals(type.getSimpleName()) // given shortName is not the class's simpleName
220+
&& instanceName.equals(simpleName(deprecatedName.oldName())) // but is the short form of the old name
221+
);
222+
}
223+
224+
private static boolean isFqName(String instanceName) {
225+
return instanceName.indexOf('.') != -1;
226+
}
129227
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/*
2+
* Copyright Kroxylicious Authors.
3+
*
4+
* Licensed under the Apache Software License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
7+
package io.kroxylicious.proxy.config;
8+
9+
import io.kroxylicious.proxy.plugin.Plugin;
10+
11+
@Plugin(configType = Void.class)
12+
@Deprecated(forRemoval = true) // not really! This class exists to test how deprecated plugins are handled
13+
public class DeprecatedImplementation implements ServiceWithBaggage {
14+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright Kroxylicious Authors.
3+
*
4+
* Licensed under the Apache Software License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
7+
package io.kroxylicious.proxy.config;
8+
9+
import io.kroxylicious.proxy.plugin.DeprecatedPluginName;
10+
import io.kroxylicious.proxy.plugin.Plugin;
11+
12+
@Plugin(configType = Void.class)
13+
@DeprecatedPluginName(oldName = "io.kroxylicious.proxy.config.ImplementationWithDeprecatedName")
14+
public class RenamedImplementation implements ServiceWithBaggage {
15+
}

0 commit comments

Comments
 (0)