Skip to content

Commit b0da473

Browse files
apuigapuig
authored
Remove jsr305 as a compile/runtime dependency and add jakarta annotation support to DataBoundConfigurator (#2798)
1 parent 8c1cf7c commit b0da473

10 files changed

Lines changed: 165 additions & 23 deletions

File tree

‎plugin/pom.xml‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
<hpi.compatibleSinceVersion>1.28</hpi.compatibleSinceVersion>
3636
<hpi.pluginChangelogUrl>https://github.com/jenkinsci/configuration-as-code-plugin/releases</hpi.pluginChangelogUrl>
3737
<hpi.pluginLogoUrl>https://raw.githubusercontent.com/jenkinsci/configuration-as-code-plugin/master/plugin/src/main/resources/images/symbols/logo.svg</hpi.pluginLogoUrl>
38-
<hpi.bundledArtifacts>jsr305,vavr,vavr-match</hpi.bundledArtifacts>
38+
<hpi.bundledArtifacts>vavr,vavr-match</hpi.bundledArtifacts>
3939
<hpi.strictBundledArtifacts>true</hpi.strictBundledArtifacts>
4040
</properties>
4141

@@ -52,14 +52,6 @@
5252
</dependencyManagement>
5353

5454
<dependencies>
55-
<dependency>
56-
<!-- Dependency is needed for plugins with DataBoundConstructors that use JSR-305 Nonnull annotation -->
57-
<!-- See the tests inside the plugin that confirm those annotations are honored -->
58-
<!-- https://github.com/jenkinsci/configuration-as-code-plugin/issues/2491 caused by removing it -->
59-
<groupId>com.google.code.findbugs</groupId>
60-
<artifactId>jsr305</artifactId>
61-
<version>3.0.2</version>
62-
</dependency>
6355
<dependency>
6456
<groupId>io.jenkins.plugins</groupId>
6557
<artifactId>caffeine-api</artifactId>
@@ -101,6 +93,12 @@
10193
<version>1.19.0</version>
10294
<scope>test</scope>
10395
</dependency>
96+
<dependency>
97+
<groupId>com.google.code.findbugs</groupId>
98+
<artifactId>jsr305</artifactId>
99+
<version>3.0.2</version>
100+
<scope>test</scope>
101+
</dependency>
104102
</dependencies>
105103

106104
<build>

‎plugin/src/main/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfigurator.java‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.jenkins.plugins.casc.impl.configurators;
22

3+
import edu.umd.cs.findbugs.annotations.CheckForNull;
34
import edu.umd.cs.findbugs.annotations.NonNull;
45
import hudson.model.Descriptor;
56
import hudson.util.Secret;
@@ -11,6 +12,8 @@
1112
import io.jenkins.plugins.casc.impl.attributes.DescribableAttribute;
1213
import io.jenkins.plugins.casc.model.CNode;
1314
import io.jenkins.plugins.casc.model.Mapping;
15+
import java.lang.annotation.Annotation;
16+
import java.lang.reflect.AnnotatedElement;
1417
import java.lang.reflect.Constructor;
1518
import java.lang.reflect.InvocationTargetException;
1619
import java.lang.reflect.Method;
@@ -25,18 +28,14 @@
2528
import java.util.Set;
2629
import java.util.logging.Level;
2730
import java.util.logging.Logger;
28-
import javax.annotation.CheckForNull;
29-
import javax.annotation.Nonnull;
30-
import javax.annotation.ParametersAreNonnullByDefault;
31-
import javax.annotation.PostConstruct;
3231
import jenkins.model.Jenkins;
3332
import org.kohsuke.stapler.ClassDescriptor;
3433
import org.kohsuke.stapler.DataBoundConstructor;
3534
import org.kohsuke.stapler.Stapler;
3635

3736
/**
3837
* A generic {@link Configurator} to configure components with a {@link org.kohsuke.stapler.DataBoundConstructor}.
39-
* Intended to replicate Stapler's request-to-instance lifecycle, including {@link PostConstruct} init methods.
38+
* Intended to replicate Stapler's request-to-instance lifecycle, including PostConstruct init methods.
4039
* Will rely on <a href="https://github.com/jenkinsci/jep/tree/master/jep/205">JEP-205</a> once implemented
4140
*
4241
* @author <a href="mailto:[email protected]">Nicolas De Loof</a>
@@ -45,6 +44,25 @@ public class DataBoundConfigurator<T> extends BaseConfigurator<T> {
4544

4645
private static final Logger LOGGER = Logger.getLogger(DataBoundConfigurator.class.getName());
4746

47+
// Avoid compile-time dependencies
48+
private static final Set<String> NONNULL_ANNOTATIONS =
49+
Set.of("jakarta.annotation.Nonnull", "javax.annotation.Nonnull");
50+
private static final Set<String> NULLABLE_ANNOTATIONS =
51+
Set.of("jakarta.annotation.Nullable", "javax.annotation.CheckForNull");
52+
private static final Set<String> PARAMETERS_ARE_NONNULL_ANNOTATIONS =
53+
Set.of("javax.annotation.ParametersAreNonnullByDefault");
54+
private static final Set<String> POST_CONSTRUCT_ANNOTATIONS =
55+
Set.of("jakarta.annotation.PostConstruct", "javax.annotation.PostConstruct");
56+
57+
private static boolean hasAnnotation(AnnotatedElement element, Set<String> annotationNames) {
58+
for (Annotation ann : element.getAnnotations()) {
59+
if (annotationNames.contains(ann.annotationType().getName())) {
60+
return true;
61+
}
62+
}
63+
return false;
64+
}
65+
4866
private final Class<T> target;
4967

5068
public DataBoundConfigurator(Class<T> clazz) {
@@ -81,7 +99,7 @@ public T configure(CNode c, ConfigurationContext context) throws ConfiguratorExc
8199
T object = super.configure(c, context);
82100

83101
for (Method method : target.getMethods()) {
84-
if (method.getParameterCount() == 0 && method.getAnnotation(PostConstruct.class) != null) {
102+
if (method.getParameterCount() == 0 && hasAnnotation(method, POST_CONSTRUCT_ANNOTATIONS)) {
85103
try {
86104
method.invoke(object, null);
87105
} catch (IllegalAccessException | InvocationTargetException e) {
@@ -115,11 +133,11 @@ private T tryConstructor(Constructor<T> constructor, Mapping config, Configurati
115133

116134
Class<?> clazz = constructor.getDeclaringClass();
117135
if (value == null
118-
&& (parameters[i].isAnnotationPresent(Nonnull.class)
119-
|| constructor.isAnnotationPresent(ParametersAreNonnullByDefault.class)
120-
|| clazz.isAnnotationPresent(ParametersAreNonnullByDefault.class)
121-
|| clazz.getPackage().isAnnotationPresent(ParametersAreNonnullByDefault.class)
122-
&& !parameters[i].isAnnotationPresent(CheckForNull.class))) {
136+
&& (hasAnnotation(parameters[i], NONNULL_ANNOTATIONS)
137+
|| hasAnnotation(constructor, PARAMETERS_ARE_NONNULL_ANNOTATIONS)
138+
|| hasAnnotation(clazz, PARAMETERS_ARE_NONNULL_ANNOTATIONS)
139+
|| hasAnnotation(clazz.getPackage(), PARAMETERS_ARE_NONNULL_ANNOTATIONS)
140+
&& !hasAnnotation(parameters[i], NULLABLE_ANNOTATIONS))) {
123141

124142
if (Set.class.isAssignableFrom(t)) {
125143
LOGGER.log(

‎plugin/src/main/java/io/jenkins/plugins/casc/yaml/MergeStrategyAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package io.jenkins.plugins.casc.yaml;
22

3+
import edu.umd.cs.findbugs.annotations.CheckForNull;
34
import hudson.Extension;
45
import hudson.ExtensionList;
56
import hudson.model.Api;
67
import hudson.model.UnprotectedRootAction;
78
import hudson.util.HttpResponses;
8-
import javax.annotation.CheckForNull;
99
import jenkins.model.Jenkins;
1010
import net.sf.json.JSONArray;
1111
import org.jenkinsci.Symbol;

‎plugin/src/main/java/io/jenkins/plugins/casc/yaml/MergeStrategyFactory.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package io.jenkins.plugins.casc.yaml;
22

3+
import edu.umd.cs.findbugs.annotations.NonNull;
34
import hudson.ExtensionList;
45
import java.util.Optional;
5-
import javax.annotation.Nonnull;
66
import jenkins.model.Jenkins;
77
import org.apache.commons.lang3.StringUtils;
88

99
public class MergeStrategyFactory {
10-
private static MergeStrategy getMergeStrategy(@Nonnull String name) {
10+
private static MergeStrategy getMergeStrategy(@NonNull String name) {
1111
ExtensionList<MergeStrategy> mergeStrategyList = Jenkins.get().getExtensionList(MergeStrategy.class);
1212
Optional<MergeStrategy> opt = mergeStrategyList.stream()
1313
.filter(strategy -> strategy.getName().equals(name))

‎test-harness/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@
6969
<groupId>org.jenkins-ci.plugins</groupId>
7070
<artifactId>jackson2-api</artifactId>
7171
</dependency>
72+
<dependency>
73+
<groupId>com.google.code.findbugs</groupId>
74+
<artifactId>jsr305</artifactId>
75+
<version>3.0.2</version>
76+
<scope>test</scope>
77+
</dependency>
7278
</dependencies>
7379

7480
<build>

‎test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/DataBoundConfiguratorTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
import io.jenkins.plugins.casc.ConfiguratorException;
2121
import io.jenkins.plugins.casc.ConfiguratorRegistry;
2222
import io.jenkins.plugins.casc.impl.configurators.nonnull.ClassParametersAreNonnullByDefault;
23+
import io.jenkins.plugins.casc.impl.configurators.nonnull.JakartaNonnullListParameterConstructor;
24+
import io.jenkins.plugins.casc.impl.configurators.nonnull.JakartaNonnullParameterConstructor;
25+
import io.jenkins.plugins.casc.impl.configurators.nonnull.JakartaNonnullRequiredParameterConstructor;
26+
import io.jenkins.plugins.casc.impl.configurators.nonnull.JakartaNullableParameterConstructor;
2327
import io.jenkins.plugins.casc.impl.configurators.nonnull.NonnullParameterConstructor;
2428
import io.jenkins.plugins.casc.impl.configurators.nonnull.nonnullparampackage.PackageParametersAreNonnullByDefault;
2529
import io.jenkins.plugins.casc.impl.configurators.nonnull.nonnullparampackage.PackageParametersNonNullCheckForNull;
@@ -185,6 +189,51 @@ void packageParametersAreNonnullByDefaultButCanBeNullable() throws Exception {
185189
assertNull(configured.getSecret());
186190
}
187191

192+
@Test
193+
void jakartaNonnullConstructorParameter() throws Exception {
194+
Mapping config = new Mapping();
195+
ConfiguratorRegistry registry = ConfiguratorRegistry.get();
196+
final JakartaNonnullParameterConstructor configured =
197+
(JakartaNonnullParameterConstructor) registry.lookupOrFail(JakartaNonnullParameterConstructor.class)
198+
.configure(config, new ConfigurationContext(registry));
199+
assertEquals(0, configured.getStrings().size());
200+
}
201+
202+
@Test
203+
void jakartaNonnullListConstructorParameter() throws Exception {
204+
Mapping config = new Mapping();
205+
ConfiguratorRegistry registry = ConfiguratorRegistry.get();
206+
final JakartaNonnullListParameterConstructor configured = (JakartaNonnullListParameterConstructor)
207+
registry.lookupOrFail(JakartaNonnullListParameterConstructor.class)
208+
.configure(config, new ConfigurationContext(registry));
209+
assertTrue(configured.getStrings().isEmpty());
210+
}
211+
212+
@Test
213+
void jakartaNonnullRequiredParameter() {
214+
Mapping config = new Mapping();
215+
ConfiguratorRegistry registry = ConfiguratorRegistry.get();
216+
217+
ConfiguratorException exception = assertThrows(ConfiguratorException.class, () -> registry.lookupOrFail(
218+
JakartaNonnullRequiredParameterConstructor.class)
219+
.configure(config, new ConfigurationContext(registry)));
220+
221+
assertThat(
222+
exception.getMessage(),
223+
is(
224+
"string is required to configure class io.jenkins.plugins.casc.impl.configurators.nonnull.JakartaNonnullRequiredParameterConstructor"));
225+
}
226+
227+
@Test
228+
void jakartaNullableConstructorParameter() throws Exception {
229+
Mapping config = new Mapping();
230+
ConfiguratorRegistry registry = ConfiguratorRegistry.get();
231+
final JakartaNullableParameterConstructor configured =
232+
(JakartaNullableParameterConstructor) registry.lookupOrFail(JakartaNullableParameterConstructor.class)
233+
.configure(config, new ConfigurationContext(registry));
234+
assertNull(configured.getSecret());
235+
}
236+
188237
@Test
189238
@SuppressWarnings("unchecked")
190239
void exportWithSets() throws Exception {

‎test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/nonnull/JakartaNonnullListParameterConstructor.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package io.jenkins.plugins.casc.impl.configurators.nonnull;
2+
3+
import jakarta.annotation.Nonnull;
4+
import java.util.List;
5+
import org.kohsuke.stapler.DataBoundConstructor;
6+
7+
public class JakartaNonnullListParameterConstructor {
8+
private List<String> strings;
9+
10+
@DataBoundConstructor
11+
public JakartaNonnullListParameterConstructor(@Nonnull List<String> strings) {
12+
this.strings = strings;
13+
}
14+
15+
public List<String> getStrings() {
16+
return strings;
17+
}
18+
}

‎test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/nonnull/JakartaNonnullParameterConstructor.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package io.jenkins.plugins.casc.impl.configurators.nonnull;
2+
3+
import jakarta.annotation.Nonnull;
4+
import java.util.Set;
5+
import org.kohsuke.stapler.DataBoundConstructor;
6+
7+
public class JakartaNonnullParameterConstructor {
8+
private Set<String> strings;
9+
10+
@DataBoundConstructor
11+
public JakartaNonnullParameterConstructor(@Nonnull Set<String> strings) {
12+
this.strings = strings;
13+
}
14+
15+
public Set<String> getStrings() {
16+
return strings;
17+
}
18+
}

‎test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/nonnull/JakartaNonnullRequiredParameterConstructor.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.jenkins.plugins.casc.impl.configurators.nonnull;
2+
3+
import jakarta.annotation.Nonnull;
4+
import org.kohsuke.stapler.DataBoundConstructor;
5+
6+
public class JakartaNonnullRequiredParameterConstructor {
7+
private String string;
8+
9+
@DataBoundConstructor
10+
public JakartaNonnullRequiredParameterConstructor(@Nonnull String string) {
11+
this.string = string;
12+
}
13+
14+
public String getString() {
15+
return string;
16+
}
17+
}

‎test-harness/src/test/java/io/jenkins/plugins/casc/impl/configurators/nonnull/JakartaNullableParameterConstructor.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package io.jenkins.plugins.casc.impl.configurators.nonnull;
2+
3+
import hudson.util.Secret;
4+
import jakarta.annotation.Nullable;
5+
import org.kohsuke.stapler.DataBoundConstructor;
6+
7+
public class JakartaNullableParameterConstructor {
8+
private Secret secret;
9+
10+
@DataBoundConstructor
11+
public JakartaNullableParameterConstructor(@Nullable Secret secret) {
12+
this.secret = secret;
13+
}
14+
15+
public Secret getSecret() {
16+
return secret;
17+
}
18+
}

0 commit comments

Comments
 (0)