Skip to content

Commit d32d9f5

Browse files
rzo1rzo1
committed
Respect DD values when merging concurrency resource annotations
AnnotationDeployer#buildManaged{Executor,ScheduledExecutor,ThreadFactory}Definition was unconditionally overwriting deployment-descriptor values (name, context, hungTaskThreshold, maxAsync, virtual, priority) with annotation values whenever both sources shared a resource name. That violates the Jakarta EE platform DD-wins rule (§EE.5.2.5); only qualifier was guarded. Guard every field assignment with a null check, mirroring the existing buildContextServiceDefinition pattern, so DD values are preserved and the annotation only fills attributes the descriptor left unset.
1 parent dff924c commit d32d9f5

2 files changed

Lines changed: 328 additions & 20 deletions

File tree

‎container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java‎

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4140,13 +4140,29 @@ private void buildManagedExecutorDefinition(final JndiConsumer consumer, final M
41404140
ManagedExecutor existing = consumer.getManagedExecutorMap().get(definition.name());
41414141
final ManagedExecutor managedExecutor = (existing != null) ? existing : new ManagedExecutor();
41424142

4143-
managedExecutor.setName(new JndiName());
4144-
managedExecutor.getName().setvalue(definition.name());
4145-
managedExecutor.setContextService(new JndiName());
4146-
managedExecutor.getContextService().setvalue(definition.context());
4147-
managedExecutor.setHungTaskThreshold(definition.hungTaskThreshold());
4148-
managedExecutor.setMaxAsync(definition.maxAsync() == -1 ? null : definition.maxAsync());
4149-
managedExecutor.setVirtual(definition.virtual() ? Boolean.TRUE : null);
4143+
if (managedExecutor.getName() == null) {
4144+
final JndiName jndiName = new JndiName();
4145+
jndiName.setvalue(definition.name());
4146+
managedExecutor.setName(jndiName);
4147+
}
4148+
4149+
if (managedExecutor.getContextService() == null) {
4150+
final JndiName contextName = new JndiName();
4151+
contextName.setvalue(definition.context());
4152+
managedExecutor.setContextService(contextName);
4153+
}
4154+
4155+
if (managedExecutor.getHungTaskThreshold() == null) {
4156+
managedExecutor.setHungTaskThreshold(definition.hungTaskThreshold());
4157+
}
4158+
4159+
if (managedExecutor.getMaxAsync() == null) {
4160+
managedExecutor.setMaxAsync(definition.maxAsync() == -1 ? null : definition.maxAsync());
4161+
}
4162+
4163+
if (managedExecutor.getVirtual() == null) {
4164+
managedExecutor.setVirtual(definition.virtual() ? Boolean.TRUE : null);
4165+
}
41504166

41514167
if (managedExecutor.getQualifier().isEmpty() && definition.qualifiers().length > 0) {
41524168
for (final Class<?> qualifier : definition.qualifiers()) {
@@ -4161,13 +4177,29 @@ private void buildManagedScheduledExecutorDefinition(final JndiConsumer consumer
41614177
ManagedScheduledExecutor existing = consumer.getManagedScheduledExecutorMap().get(definition.name());
41624178
final ManagedScheduledExecutor managedScheduledExecutor = (existing != null) ? existing : new ManagedScheduledExecutor();
41634179

4164-
managedScheduledExecutor.setName(new JndiName());
4165-
managedScheduledExecutor.getName().setvalue(definition.name());
4166-
managedScheduledExecutor.setContextService(new JndiName());
4167-
managedScheduledExecutor.getContextService().setvalue(definition.context());
4168-
managedScheduledExecutor.setHungTaskThreshold(definition.hungTaskThreshold());
4169-
managedScheduledExecutor.setMaxAsync(definition.maxAsync() == -1 ? null : definition.maxAsync());
4170-
managedScheduledExecutor.setVirtual(definition.virtual() ? Boolean.TRUE : null);
4180+
if (managedScheduledExecutor.getName() == null) {
4181+
final JndiName jndiName = new JndiName();
4182+
jndiName.setvalue(definition.name());
4183+
managedScheduledExecutor.setName(jndiName);
4184+
}
4185+
4186+
if (managedScheduledExecutor.getContextService() == null) {
4187+
final JndiName contextName = new JndiName();
4188+
contextName.setvalue(definition.context());
4189+
managedScheduledExecutor.setContextService(contextName);
4190+
}
4191+
4192+
if (managedScheduledExecutor.getHungTaskThreshold() == null) {
4193+
managedScheduledExecutor.setHungTaskThreshold(definition.hungTaskThreshold());
4194+
}
4195+
4196+
if (managedScheduledExecutor.getMaxAsync() == null) {
4197+
managedScheduledExecutor.setMaxAsync(definition.maxAsync() == -1 ? null : definition.maxAsync());
4198+
}
4199+
4200+
if (managedScheduledExecutor.getVirtual() == null) {
4201+
managedScheduledExecutor.setVirtual(definition.virtual() ? Boolean.TRUE : null);
4202+
}
41714203

41724204
if (managedScheduledExecutor.getQualifier().isEmpty() && definition.qualifiers().length > 0) {
41734205
for (final Class<?> qualifier : definition.qualifiers()) {
@@ -4182,12 +4214,25 @@ private void buildManagedThreadFactoryDefinition(final JndiConsumer consumer, Ma
41824214
ManagedThreadFactory existing = consumer.getManagedThreadFactoryMap().get(definition.name());
41834215
final ManagedThreadFactory managedThreadFactory = (existing != null) ? existing : new ManagedThreadFactory();
41844216

4185-
managedThreadFactory.setName(new JndiName());
4186-
managedThreadFactory.getName().setvalue(definition.name());
4187-
managedThreadFactory.setContextService(new JndiName());
4188-
managedThreadFactory.getContextService().setvalue(definition.context());
4189-
managedThreadFactory.setPriority(definition.priority());
4190-
managedThreadFactory.setVirtual(definition.virtual() ? Boolean.TRUE : null);
4217+
if (managedThreadFactory.getName() == null) {
4218+
final JndiName jndiName = new JndiName();
4219+
jndiName.setvalue(definition.name());
4220+
managedThreadFactory.setName(jndiName);
4221+
}
4222+
4223+
if (managedThreadFactory.getContextService() == null) {
4224+
final JndiName contextName = new JndiName();
4225+
contextName.setvalue(definition.context());
4226+
managedThreadFactory.setContextService(contextName);
4227+
}
4228+
4229+
if (managedThreadFactory.getPriority() == null) {
4230+
managedThreadFactory.setPriority(definition.priority());
4231+
}
4232+
4233+
if (managedThreadFactory.getVirtual() == null) {
4234+
managedThreadFactory.setVirtual(definition.virtual() ? Boolean.TRUE : null);
4235+
}
41914236

41924237
if (managedThreadFactory.getQualifier().isEmpty() && definition.qualifiers().length > 0) {
41934238
for (final Class<?> qualifier : definition.qualifiers()) {

‎container/openejb-core/src/test/java/org/apache/openejb/config/ConcurrencyDefinitionDDOverrideTest.java‎

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.openejb.config;
18+
19+
import jakarta.enterprise.concurrent.ContextServiceDefinition;
20+
import jakarta.enterprise.concurrent.ManagedExecutorDefinition;
21+
import jakarta.enterprise.concurrent.ManagedScheduledExecutorDefinition;
22+
import jakarta.enterprise.concurrent.ManagedThreadFactoryDefinition;
23+
import jakarta.enterprise.util.Nonbinding;
24+
import jakarta.inject.Qualifier;
25+
import org.apache.openejb.jee.ContextService;
26+
import org.apache.openejb.jee.ManagedExecutor;
27+
import org.apache.openejb.jee.ManagedScheduledExecutor;
28+
import org.apache.openejb.jee.ManagedThreadFactory;
29+
import org.apache.openejb.jee.StatelessBean;
30+
import org.apache.openejb.jee.jba.JndiName;
31+
import org.apache.xbean.finder.AnnotationFinder;
32+
import org.apache.xbean.finder.archive.ClassesArchive;
33+
import org.junit.Test;
34+
35+
import java.lang.annotation.ElementType;
36+
import java.lang.annotation.Retention;
37+
import java.lang.annotation.RetentionPolicy;
38+
import java.lang.annotation.Target;
39+
import java.util.List;
40+
41+
import static org.junit.Assert.assertEquals;
42+
import static org.junit.Assert.assertTrue;
43+
44+
/**
45+
* Verifies {@link AnnotationDeployer.ProcessAnnotatedBeans#buildAnnotatedRefs} applies the
46+
* DD-wins merging rule (Jakarta EE platform §EE.5.2.5) when an annotation and a deployment
47+
* descriptor entry share a name: the DD value is retained for every attribute, and the
48+
* annotation only fills attributes the DD left unset.
49+
*/
50+
public class ConcurrencyDefinitionDDOverrideTest {
51+
52+
// ---------------- ContextService ----------------
53+
54+
@Test
55+
public void contextServiceDDOverridesAnnotation() throws Exception {
56+
final StatelessBean bean = new StatelessBean("Bean", CSBean.class.getName());
57+
58+
final ContextService dd = new ContextService();
59+
dd.setName(jndi("java:app/concurrent/CS"));
60+
dd.getPropagated().add("StringContext");
61+
dd.getCleared().add("IntContext");
62+
dd.getUnchanged().add("Transaction");
63+
dd.getQualifier().add(DDQualifier.class.getName());
64+
bean.getContextServiceMap().put("java:app/concurrent/CS", dd);
65+
66+
buildAnnotatedRefs(bean, CSBean.class);
67+
68+
final ContextService merged = bean.getContextServiceMap().get("java:app/concurrent/CS");
69+
assertEquals("java:app/concurrent/CS", merged.getName().getvalue());
70+
assertEquals(List.of("StringContext"), merged.getPropagated());
71+
assertEquals(List.of("IntContext"), merged.getCleared());
72+
assertEquals(List.of("Transaction"), merged.getUnchanged());
73+
assertEquals(List.of(DDQualifier.class.getName()), merged.getQualifier());
74+
}
75+
76+
@Test
77+
public void contextServiceAnnotationFillsMissingDDFields() throws Exception {
78+
final StatelessBean bean = new StatelessBean("Bean", CSBean.class.getName());
79+
buildAnnotatedRefs(bean, CSBean.class);
80+
81+
final ContextService merged = bean.getContextServiceMap().get("java:app/concurrent/CS");
82+
assertEquals(List.of(ContextServiceDefinition.APPLICATION), merged.getPropagated());
83+
assertEquals(List.of(ContextServiceDefinition.TRANSACTION), merged.getCleared());
84+
assertTrue(merged.getUnchanged().isEmpty());
85+
assertEquals(List.of(AnnoQualifier.class.getName()), merged.getQualifier());
86+
}
87+
88+
// ---------------- ManagedExecutor ----------------
89+
90+
@Test
91+
public void managedExecutorDDOverridesAnnotation() throws Exception {
92+
final StatelessBean bean = new StatelessBean("Bean", MEBean.class.getName());
93+
94+
final ManagedExecutor dd = new ManagedExecutor();
95+
dd.setName(jndi("java:app/concurrent/MES"));
96+
dd.setContextService(jndi("java:app/concurrent/DDCtx"));
97+
dd.setHungTaskThreshold(100_000L);
98+
dd.setMaxAsync(5);
99+
dd.setVirtual(Boolean.FALSE);
100+
dd.getQualifier().add(DDQualifier.class.getName());
101+
bean.getManagedExecutorMap().put("java:app/concurrent/MES", dd);
102+
103+
buildAnnotatedRefs(bean, MEBean.class);
104+
105+
final ManagedExecutor merged = bean.getManagedExecutorMap().get("java:app/concurrent/MES");
106+
assertEquals("java:app/concurrent/MES", merged.getName().getvalue());
107+
assertEquals("java:app/concurrent/DDCtx", merged.getContextService().getvalue());
108+
assertEquals(Long.valueOf(100_000L), merged.getHungTaskThreshold());
109+
assertEquals(Integer.valueOf(5), merged.getMaxAsync());
110+
assertEquals(Boolean.FALSE, merged.getVirtual());
111+
assertEquals(List.of(DDQualifier.class.getName()), merged.getQualifier());
112+
}
113+
114+
@Test
115+
public void managedExecutorAnnotationFillsMissingDDFields() throws Exception {
116+
final StatelessBean bean = new StatelessBean("Bean", MEBean.class.getName());
117+
buildAnnotatedRefs(bean, MEBean.class);
118+
119+
final ManagedExecutor merged = bean.getManagedExecutorMap().get("java:app/concurrent/MES");
120+
assertEquals("java:app/concurrent/AnnoCtx", merged.getContextService().getvalue());
121+
assertEquals(Long.valueOf(200_000L), merged.getHungTaskThreshold());
122+
assertEquals(Integer.valueOf(3), merged.getMaxAsync());
123+
assertEquals(Boolean.TRUE, merged.getVirtual());
124+
assertEquals(List.of(AnnoQualifier.class.getName()), merged.getQualifier());
125+
}
126+
127+
// ---------------- ManagedScheduledExecutor ----------------
128+
129+
@Test
130+
public void managedScheduledExecutorDDOverridesAnnotation() throws Exception {
131+
final StatelessBean bean = new StatelessBean("Bean", MSESBean.class.getName());
132+
133+
final ManagedScheduledExecutor dd = new ManagedScheduledExecutor();
134+
dd.setName(jndi("java:app/concurrent/MSES"));
135+
dd.setContextService(jndi("java:app/concurrent/DDCtx"));
136+
dd.setHungTaskThreshold(150_000L);
137+
dd.setMaxAsync(4);
138+
dd.setVirtual(Boolean.FALSE);
139+
dd.getQualifier().add(DDQualifier.class.getName());
140+
bean.getManagedScheduledExecutorMap().put("java:app/concurrent/MSES", dd);
141+
142+
buildAnnotatedRefs(bean, MSESBean.class);
143+
144+
final ManagedScheduledExecutor merged = bean.getManagedScheduledExecutorMap().get("java:app/concurrent/MSES");
145+
assertEquals("java:app/concurrent/DDCtx", merged.getContextService().getvalue());
146+
assertEquals(Long.valueOf(150_000L), merged.getHungTaskThreshold());
147+
assertEquals(Integer.valueOf(4), merged.getMaxAsync());
148+
assertEquals(Boolean.FALSE, merged.getVirtual());
149+
assertEquals(List.of(DDQualifier.class.getName()), merged.getQualifier());
150+
}
151+
152+
@Test
153+
public void managedScheduledExecutorAnnotationFillsMissingDDFields() throws Exception {
154+
final StatelessBean bean = new StatelessBean("Bean", MSESBean.class.getName());
155+
buildAnnotatedRefs(bean, MSESBean.class);
156+
157+
final ManagedScheduledExecutor merged = bean.getManagedScheduledExecutorMap().get("java:app/concurrent/MSES");
158+
assertEquals("java:app/concurrent/AnnoCtx", merged.getContextService().getvalue());
159+
assertEquals(Long.valueOf(250_000L), merged.getHungTaskThreshold());
160+
assertEquals(Integer.valueOf(2), merged.getMaxAsync());
161+
assertEquals(Boolean.TRUE, merged.getVirtual());
162+
assertEquals(List.of(AnnoQualifier.class.getName()), merged.getQualifier());
163+
}
164+
165+
// ---------------- ManagedThreadFactory ----------------
166+
167+
@Test
168+
public void managedThreadFactoryDDOverridesAnnotation() throws Exception {
169+
final StatelessBean bean = new StatelessBean("Bean", MTFBean.class.getName());
170+
171+
final ManagedThreadFactory dd = new ManagedThreadFactory();
172+
dd.setName(jndi("java:app/concurrent/MTF"));
173+
dd.setContextService(jndi("java:app/concurrent/DDCtx"));
174+
dd.setPriority(7);
175+
dd.setVirtual(Boolean.FALSE);
176+
dd.getQualifier().add(DDQualifier.class.getName());
177+
bean.getManagedThreadFactoryMap().put("java:app/concurrent/MTF", dd);
178+
179+
buildAnnotatedRefs(bean, MTFBean.class);
180+
181+
final ManagedThreadFactory merged = bean.getManagedThreadFactoryMap().get("java:app/concurrent/MTF");
182+
assertEquals("java:app/concurrent/DDCtx", merged.getContextService().getvalue());
183+
assertEquals(Integer.valueOf(7), merged.getPriority());
184+
assertEquals(Boolean.FALSE, merged.getVirtual());
185+
assertEquals(List.of(DDQualifier.class.getName()), merged.getQualifier());
186+
}
187+
188+
@Test
189+
public void managedThreadFactoryAnnotationFillsMissingDDFields() throws Exception {
190+
final StatelessBean bean = new StatelessBean("Bean", MTFBean.class.getName());
191+
buildAnnotatedRefs(bean, MTFBean.class);
192+
193+
final ManagedThreadFactory merged = bean.getManagedThreadFactoryMap().get("java:app/concurrent/MTF");
194+
assertEquals("java:app/concurrent/AnnoCtx", merged.getContextService().getvalue());
195+
assertEquals(Integer.valueOf(8), merged.getPriority());
196+
assertEquals(Boolean.TRUE, merged.getVirtual());
197+
assertEquals(List.of(AnnoQualifier.class.getName()), merged.getQualifier());
198+
}
199+
200+
// ---------------- helpers ----------------
201+
202+
private static void buildAnnotatedRefs(final StatelessBean bean, final Class<?> beanClass) throws Exception {
203+
final AnnotationFinder finder = new AnnotationFinder(new ClassesArchive(beanClass)).link();
204+
new AnnotationDeployer.ProcessAnnotatedBeans(false)
205+
.buildAnnotatedRefs(bean, finder, beanClass.getClassLoader());
206+
}
207+
208+
private static JndiName jndi(final String value) {
209+
final JndiName n = new JndiName();
210+
n.setvalue(value);
211+
return n;
212+
}
213+
214+
@Qualifier
215+
@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER })
216+
@Retention(RetentionPolicy.RUNTIME)
217+
public @interface DDQualifier {
218+
}
219+
220+
@Qualifier
221+
@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER })
222+
@Retention(RetentionPolicy.RUNTIME)
223+
public @interface AnnoQualifier {
224+
@Nonbinding String value() default "";
225+
}
226+
227+
@ContextServiceDefinition(
228+
name = "java:app/concurrent/CS",
229+
propagated = ContextServiceDefinition.APPLICATION,
230+
cleared = ContextServiceDefinition.TRANSACTION,
231+
qualifiers = AnnoQualifier.class)
232+
public static class CSBean {
233+
}
234+
235+
@ManagedExecutorDefinition(
236+
name = "java:app/concurrent/MES",
237+
context = "java:app/concurrent/AnnoCtx",
238+
hungTaskThreshold = 200_000,
239+
maxAsync = 3,
240+
virtual = true,
241+
qualifiers = AnnoQualifier.class)
242+
public static class MEBean {
243+
}
244+
245+
@ManagedScheduledExecutorDefinition(
246+
name = "java:app/concurrent/MSES",
247+
context = "java:app/concurrent/AnnoCtx",
248+
hungTaskThreshold = 250_000,
249+
maxAsync = 2,
250+
virtual = true,
251+
qualifiers = AnnoQualifier.class)
252+
public static class MSESBean {
253+
}
254+
255+
@ManagedThreadFactoryDefinition(
256+
name = "java:app/concurrent/MTF",
257+
context = "java:app/concurrent/AnnoCtx",
258+
priority = 8,
259+
virtual = true,
260+
qualifiers = AnnoQualifier.class)
261+
public static class MTFBean {
262+
}
263+
}

0 commit comments

Comments
 (0)