Skip to content

Commit ebe3f01

Browse files
ehsavoieehsavoie
committed
fix(multitenancy): support @PublicAgentCard on tenant-specific cards
- @tenant + @PublicAgentCard takes precedence over bare @tenant cards - Reject duplicate tenant card registrations - Throw when no default AgentExecutor bean exists - Widen A2AServerRoutes catch to handle all server errors Fixes #1175 Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
1 parent ec5994a commit ebe3f01

7 files changed

Lines changed: 223 additions & 20 deletions

File tree

‎extras/multitenancy/src/main/java/org/a2aproject/sdk/extras/multitenancy/CdiAgentCardRouter.java‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,17 @@
2020
* CDI-based {@link AgentCardRouter} that resolves tenant-specific {@link AgentCard} beans.
2121
* <p>
2222
* Extended cards are resolved via {@code @Tenant("x") @ExtendedAgentCard}-qualified beans.
23-
* Tenant-specific public cards are resolved via {@code @Tenant("x")}-qualified beans that
24-
* carry neither {@code @ExtendedAgentCard} nor {@code @PublicAgentCard}. The {@code @PublicAgentCard}
25-
* qualifier must NOT be used on tenant-specific public cards because it would cause CDI
26-
* ambiguity on injection points requesting the default public card.
23+
* Tenant-specific public cards are resolved by matching beans that carry the
24+
* {@code @Tenant("x")} qualifier and either:
25+
* <ul>
26+
* <li>carry {@code @PublicAgentCard} (preferred — these cards are resolved before bare
27+
* tenant cards, even if they also carry {@code @ExtendedAgentCard}), or</li>
28+
* <li>carry neither {@code @ExtendedAgentCard} nor {@code @PublicAgentCard} (bare
29+
* tenant card).</li>
30+
* </ul>
31+
* Using {@code @Tenant("x") @PublicAgentCard} on a tenant-specific bean is safe because
32+
* {@link org.a2aproject.sdk.server.util.CdiUtils#resolveDefaultBean} excludes
33+
* {@code @Tenant}-qualified beans when resolving the default public card.
2734
* <p>
2835
* Returns the default public card from {@link #resolvePublicCard} when the tenant is
2936
* {@code null} or blank. Returns {@code null} when a non-blank tenant does not match
@@ -65,16 +72,31 @@ void init() {
6572
if (tenant == null || tenant.isBlank()) {
6673
return defaultPublicCard;
6774
}
75+
AgentCard explicitPublicCard = null;
76+
AgentCard bareCard = null;
6877
for (Instance.Handle<AgentCard> handle : allCards.handles()) {
6978
Set<Annotation> qualifiers = handle.getBean().getQualifiers();
7079
boolean matchesTenant = qualifiers.stream()
7180
.anyMatch(a -> a instanceof Tenant t && tenant.equals(t.value()));
72-
if (matchesTenant
73-
&& qualifiers.stream().noneMatch(ExtendedAgentCard.class::isInstance)
74-
&& qualifiers.stream().noneMatch(PublicAgentCard.class::isInstance)) {
75-
return handle.get();
81+
if (!matchesTenant) {
82+
continue;
83+
}
84+
boolean hasPublic = qualifiers.stream().anyMatch(PublicAgentCard.class::isInstance);
85+
boolean hasExtended = qualifiers.stream().anyMatch(ExtendedAgentCard.class::isInstance);
86+
if (hasPublic) {
87+
if (explicitPublicCard != null) {
88+
throw new IllegalStateException(String.format(
89+
"Multiple tenant @PublicAgentCard public cards detected for tenant '%s'", tenant));
90+
}
91+
explicitPublicCard = handle.get();
92+
} else if (!hasExtended) {
93+
if (bareCard != null) {
94+
throw new IllegalStateException(String.format(
95+
"Multiple tenant bare public cards detected for tenant '%s'", tenant));
96+
}
97+
bareCard = handle.get();
7698
}
7799
}
78-
return null;
100+
return explicitPublicCard != null ? explicitPublicCard : bareCard;
79101
}
80102
}

‎extras/multitenancy/src/main/java/org/a2aproject/sdk/extras/multitenancy/CdiAgentExecutorRouter.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,26 @@ void init() {
3636
@Override
3737
public AgentExecutor resolve(@Nullable String tenant) {
3838
if (tenant == null || tenant.isBlank()) {
39-
return defaultExecutor;
39+
return requireDefaultExecutor();
4040
}
4141
Instance<AgentExecutor> selected = allExecutors.select(new Tenant.Literal(tenant));
4242
if (selected.isResolvable()) {
4343
return selected.get();
4444
}
45+
return requireDefaultExecutor();
46+
}
47+
48+
/**
49+
* Returns the default executor, throwing if none was discovered at startup.
50+
*
51+
* @return the default executor, never {@code null}
52+
* @throws IllegalStateException if no default {@link AgentExecutor} bean exists
53+
*/
54+
private AgentExecutor requireDefaultExecutor() {
55+
if (defaultExecutor == null) {
56+
throw new IllegalStateException(
57+
"No default AgentExecutor bean found — provide an AgentExecutor bean without @Tenant qualifier");
58+
}
4559
return defaultExecutor;
4660
}
4761
}

‎extras/multitenancy/src/main/java/org/a2aproject/sdk/extras/multitenancy/package-info.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@NullMarked
2+
package org.a2aproject.sdk.extras.multitenancy;
3+
4+
import org.jspecify.annotations.NullMarked;
5+

‎extras/multitenancy/tests/router-unit/src/test/java/org/a2aproject/sdk/extras/multitenancy/CdiAgentCardRouterTest.java‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertNull;
5+
import static org.junit.jupiter.api.Assertions.assertThrows;
56

67
import java.util.Collections;
78
import java.util.List;
@@ -113,6 +114,51 @@ void publicCardNoDefaultReturnsNull() {
113114
assertNull(router.resolvePublicCard("unknown"));
114115
}
115116

117+
@Test
118+
void publicCardWithPublicQualifierResolvesToTenantSpecific() {
119+
startContainer(DefaultPublicAndExtendedCardProducer.class, PublicQualifiedTenantCardProducer.class);
120+
CdiAgentCardRouter router = container.select(CdiAgentCardRouter.class).get();
121+
assertEquals("acme-public-qualified", router.resolvePublicCard("acme").name());
122+
}
123+
124+
@Test
125+
void publicCardWithBothPublicAndExtendedQualifierResolvesToTenantSpecific() {
126+
startContainer(DefaultPublicAndExtendedCardProducer.class, PublicAndExtendedTenantCardProducer.class);
127+
CdiAgentCardRouter router = container.select(CdiAgentCardRouter.class).get();
128+
assertEquals("acme-both", router.resolvePublicCard("acme").name());
129+
}
130+
131+
@Test
132+
void explicitPublicCardTakesPrecedenceOverBareTenantCard() {
133+
startContainer(DefaultPublicAndExtendedCardProducer.class, BareAndPublicQualifiedTenantCardProducer.class);
134+
CdiAgentCardRouter router = container.select(CdiAgentCardRouter.class).get();
135+
assertEquals("acme-public-qualified", router.resolvePublicCard("acme").name());
136+
}
137+
138+
@Test
139+
void duplicatePublicQualifiedCardsThrows() {
140+
startContainer(DefaultPublicAndExtendedCardProducer.class, DuplicatePublicQualifiedTenantCardProducer.class);
141+
CdiAgentCardRouter router = container.select(CdiAgentCardRouter.class).get();
142+
assertThrows(IllegalStateException.class, () -> router.resolvePublicCard("acme"));
143+
}
144+
145+
@Test
146+
void duplicateBareTenantCardsThrows() {
147+
startContainer(DefaultPublicAndExtendedCardProducer.class, DuplicateBareTenantCardProducer.class);
148+
CdiAgentCardRouter router = container.select(CdiAgentCardRouter.class).get();
149+
assertThrows(IllegalStateException.class, () -> router.resolvePublicCard("acme"));
150+
}
151+
152+
@Test
153+
void dualQualifiedCardResolvedByBothPublicAndExtended() {
154+
startContainer(DefaultPublicAndExtendedCardProducer.class, PublicAndExtendedTenantCardProducer.class);
155+
CdiAgentCardRouter router = container.select(CdiAgentCardRouter.class).get();
156+
AgentCard publicCard = router.resolvePublicCard("acme");
157+
AgentCard extendedCard = router.resolveExtendedCard("acme");
158+
assertEquals("acme-both", publicCard.name());
159+
assertEquals("acme-both", extendedCard.name());
160+
}
161+
116162
private static AgentCard buildCard(String name) {
117163
return AgentCard.builder()
118164
.name(name)
@@ -179,4 +225,88 @@ AgentCard acmeExtendedCard() {
179225
return buildCard("acme-extended");
180226
}
181227
}
228+
229+
static class DefaultPublicAndExtendedCardProducer {
230+
231+
@Produces
232+
@PublicAgentCard
233+
AgentCard defaultPublicCard() {
234+
return buildCard("default-public");
235+
}
236+
237+
@Produces
238+
@ExtendedAgentCard
239+
AgentCard defaultExtendedCard() {
240+
return buildCard("default-extended");
241+
}
242+
}
243+
244+
static class PublicQualifiedTenantCardProducer {
245+
246+
@Produces
247+
@Tenant("acme")
248+
@PublicAgentCard
249+
AgentCard acmePublicCard() {
250+
return buildCard("acme-public-qualified");
251+
}
252+
}
253+
254+
static class PublicAndExtendedTenantCardProducer {
255+
256+
@Produces
257+
@Tenant("acme")
258+
@PublicAgentCard
259+
@ExtendedAgentCard
260+
AgentCard acmeBothCard() {
261+
return buildCard("acme-both");
262+
}
263+
}
264+
265+
static class BareAndPublicQualifiedTenantCardProducer {
266+
267+
@Produces
268+
@Tenant("acme")
269+
AgentCard acmeBareCard() {
270+
return buildCard("acme-bare");
271+
}
272+
273+
@Produces
274+
@Tenant("acme")
275+
@PublicAgentCard
276+
AgentCard acmePublicCard() {
277+
return buildCard("acme-public-qualified");
278+
}
279+
}
280+
281+
static class DuplicatePublicQualifiedTenantCardProducer {
282+
283+
@Produces
284+
@Tenant("acme")
285+
@PublicAgentCard
286+
AgentCard acmePublicCard1() {
287+
return buildCard("acme-public-1");
288+
}
289+
290+
@Produces
291+
@Tenant("acme")
292+
@PublicAgentCard
293+
AgentCard acmePublicCard2() {
294+
return buildCard("acme-public-2");
295+
}
296+
}
297+
298+
static class DuplicateBareTenantCardProducer {
299+
300+
@Produces
301+
@Tenant("acme")
302+
AgentCard acmeBareCard1() {
303+
return buildCard("acme-bare-1");
304+
}
305+
306+
@Produces
307+
@Tenant("acme")
308+
AgentCard acmeBareCard2() {
309+
return buildCard("acme-bare-2");
310+
}
311+
}
182312
}

‎extras/multitenancy/tests/router-unit/src/test/java/org/a2aproject/sdk/extras/multitenancy/CdiAgentExecutorRouterTest.java‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.a2aproject.sdk.extras.multitenancy;
22

33
import static org.junit.jupiter.api.Assertions.assertSame;
4+
import static org.junit.jupiter.api.Assertions.assertThrows;
45

56
import java.util.List;
67

@@ -14,52 +15,73 @@
1415
import org.a2aproject.sdk.spec.A2AError;
1516
import org.a2aproject.sdk.spec.TextPart;
1617
import org.junit.jupiter.api.AfterEach;
17-
import org.junit.jupiter.api.BeforeEach;
1818
import org.junit.jupiter.api.Test;
1919

2020
class CdiAgentExecutorRouterTest {
2121

2222
private SeContainer container;
2323

24-
@BeforeEach
25-
void startContainer() {
26-
container = SeContainerInitializer.newInstance()
24+
private void startContainer(Class<?>... beanClasses) {
25+
SeContainerInitializer initializer = SeContainerInitializer.newInstance()
2726
.disableDiscovery()
28-
.addBeanClasses(CdiAgentExecutorRouter.class, ExecutorProducer.class)
29-
.initialize();
27+
.addBeanClasses(CdiAgentExecutorRouter.class);
28+
for (Class<?> beanClass : beanClasses) {
29+
initializer.addBeanClasses(beanClass);
30+
}
31+
container = initializer.initialize();
3032
}
3133

3234
@AfterEach
3335
void closeContainer() {
34-
container.close();
36+
if (container != null) {
37+
container.close();
38+
}
3539
}
3640

3741
@Test
3842
void knownTenantResolvesToTenantSpecificExecutor() {
43+
startContainer(ExecutorProducer.class);
3944
CdiAgentExecutorRouter router = container.select(CdiAgentExecutorRouter.class).get();
4045
AgentExecutor resolved = router.resolve("acme");
4146
assertSame(ExecutorProducer.ACME, resolved);
4247
}
4348

4449
@Test
4550
void unknownTenantFallsBackToDefault() {
51+
startContainer(ExecutorProducer.class);
4652
CdiAgentExecutorRouter router = container.select(CdiAgentExecutorRouter.class).get();
4753
assertSame(ExecutorProducer.DEFAULT, router.resolve("unknown"));
4854
}
4955

5056
@Test
5157
void nullTenantReturnsDefault() {
58+
startContainer(ExecutorProducer.class);
5259
CdiAgentExecutorRouter router = container.select(CdiAgentExecutorRouter.class).get();
5360
assertSame(ExecutorProducer.DEFAULT, router.resolve(null));
5461
}
5562

5663
@Test
5764
void blankTenantReturnsDefault() {
65+
startContainer(ExecutorProducer.class);
5866
CdiAgentExecutorRouter router = container.select(CdiAgentExecutorRouter.class).get();
5967
assertSame(ExecutorProducer.DEFAULT, router.resolve(""));
6068
assertSame(ExecutorProducer.DEFAULT, router.resolve(" "));
6169
}
6270

71+
@Test
72+
void noDefaultExecutorThrowsOnNullTenant() {
73+
startContainer(TenantOnlyExecutorProducer.class);
74+
CdiAgentExecutorRouter router = container.select(CdiAgentExecutorRouter.class).get();
75+
assertThrows(IllegalStateException.class, () -> router.resolve(null));
76+
}
77+
78+
@Test
79+
void noDefaultExecutorThrowsOnUnknownTenant() {
80+
startContainer(TenantOnlyExecutorProducer.class);
81+
CdiAgentExecutorRouter router = container.select(CdiAgentExecutorRouter.class).get();
82+
assertThrows(IllegalStateException.class, () -> router.resolve("unknown"));
83+
}
84+
6385
static class ExecutorProducer {
6486

6587
static final AgentExecutor DEFAULT = new LabelExecutor("default");
@@ -77,6 +99,15 @@ AgentExecutor acmeExecutor() {
7799
}
78100
}
79101

102+
static class TenantOnlyExecutorProducer {
103+
104+
@Produces
105+
@Tenant("acme")
106+
AgentExecutor acmeExecutor() {
107+
return new LabelExecutor("acme");
108+
}
109+
}
110+
80111
static class LabelExecutor implements AgentExecutor {
81112
private final String label;
82113

‎extras/multitenancy/tests/test-common/src/main/java/org/a2aproject/sdk/extras/multitenancy/tests/MultiTenantAgentCardProducer.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@
2323
* The transport interface stamped onto every card is chosen from the {@code preferred-transport}
2424
* key in {@code /a2a-requesthandler-test.properties}, so one producer serves every transport module.
2525
*
26-
* <p>Router contract: per-tenant public cards carry {@code @Tenant} only (never {@code @PublicAgentCard}),
27-
* per-tenant extended cards carry {@code @Tenant @ExtendedAgentCard}.
26+
* <p>Router contract: per-tenant public cards carry either {@code @Tenant} alone or
27+
* {@code @Tenant @PublicAgentCard}; explicitly public cards take precedence. Per-tenant extended
28+
* cards carry {@code @Tenant @ExtendedAgentCard}.
2829
*/
2930
@Singleton
3031
public class MultiTenantAgentCardProducer {

‎reference/jsonrpc/src/main/java/org/a2aproject/sdk/server/apps/quarkus/A2AServerRoutes.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ void setupRoutes(@Observes Router router) {
263263
ctx.response().setStatusCode(404).putHeader(CONTENT_TYPE, "text/plain").end(e.getResponseMessage());
264264
} catch (IllegalArgumentException e) {
265265
ctx.response().setStatusCode(400).putHeader(CONTENT_TYPE, "text/plain").end(e.getMessage());
266-
} catch (JsonProcessingException e) {
266+
} catch (Exception e) {
267267
ctx.response().setStatusCode(500).putHeader(CONTENT_TYPE, "text/plain").end("Internal Server Error");
268268
}
269269
});

0 commit comments

Comments
 (0)