Skip to content

Commit 5246067

Browse files
tchapacanishymko
andauthored
feat: use case-insensitive transport protocol name comparison in ClientFactory (#281)
# Description Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Follow the [`CONTRIBUTING` Guide](https://github.com/google-a2a/a2a-js/blob/main/CONTRIBUTING.md). - [x] Make your Pull Request title in the <https://www.conventionalcommits.org/> specification. - Important Prefixes for [release-please](https://github.com/googleapis/release-please): - `fix:` which represents bug fixes, and correlates to a [SemVer](https://semver.org/) patch. - `feat:` represents a new feature, and correlates to a SemVer minor. - `feat!:`, or `fix!:`, `refactor!:`, etc., which represent a breaking change (indicated by the `!`) and will result in a SemVer major. - [x] Ensure the tests and linter pass - [ ] Appropriate docs were updated (if necessary) Fixes #264 🦕 Release-As: 0.3.8 --------- Co-authored-by: Ivan Shimko <vana.shimko@gmail.com>
1 parent d0e0485 commit 5246067

File tree

2 files changed

+96
-7
lines changed

2 files changed

+96
-7
lines changed

src/client/factory.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export const ClientFactoryOptions = {
7575
};
7676

7777
export class ClientFactory {
78-
private readonly transportsByName: Map<string, TransportFactory>;
78+
private readonly transportsByName: CaseInsensitiveMap<TransportFactory>;
7979
private readonly agentCardResolver: AgentCardResolver;
8080

8181
constructor(public readonly options: ClientFactoryOptions = ClientFactoryOptions.default) {
@@ -84,8 +84,7 @@ export class ClientFactory {
8484
}
8585
this.transportsByName = transportsByName(options.transports);
8686
for (const transport of options.preferredTransports ?? []) {
87-
const factory = this.options.transports.find((t) => t.protocolName === transport);
88-
if (!factory) {
87+
if (!this.transportsByName.has(transport)) {
8988
throw new Error(
9089
`Unknown preferred transport: ${transport}, available transports: ${[...this.transportsByName.keys()].join()}`
9190
);
@@ -100,7 +99,7 @@ export class ClientFactory {
10099
async createFromAgentCard(agentCard: AgentCard): Promise<Client> {
101100
const agentCardPreferred = agentCard.preferredTransport ?? JsonRpcTransportFactory.name;
102101
const additionalInterfaces = agentCard.additionalInterfaces ?? [];
103-
const urlsPerAgentTransports = new Map<string, string>([
102+
const urlsPerAgentTransports = new CaseInsensitiveMap<string>([
104103
[agentCardPreferred, agentCard.url],
105104
...additionalInterfaces.map<[string, string]>((i) => [i.transport, i.url]),
106105
]);
@@ -165,8 +164,8 @@ function mergeTransports(
165164

166165
function transportsByName(
167166
transports: ReadonlyArray<TransportFactory> | undefined
168-
): Map<string, TransportFactory> {
169-
const result = new Map<string, TransportFactory>();
167+
): CaseInsensitiveMap<TransportFactory> {
168+
const result = new CaseInsensitiveMap<TransportFactory>();
170169
if (!transports) {
171170
return result;
172171
}
@@ -189,3 +188,29 @@ function mergeArrays<T>(
189188

190189
return [...(a1 ?? []), ...(a2 ?? [])];
191190
}
191+
192+
/**
193+
* A Map that normalizes string keys to uppercase for case-insensitive lookups.
194+
* This prevents errors from inconsistent casing in protocol names.
195+
*/
196+
class CaseInsensitiveMap<T> extends Map<string, T> {
197+
private normalizeKey(key: string): string {
198+
return key.toUpperCase();
199+
}
200+
201+
override set(key: string, value: T): this {
202+
return super.set(this.normalizeKey(key), value);
203+
}
204+
205+
override get(key: string): T | undefined {
206+
return super.get(this.normalizeKey(key));
207+
}
208+
209+
override has(key: string): boolean {
210+
return super.has(this.normalizeKey(key));
211+
}
212+
213+
override delete(key: string): boolean {
214+
return super.delete(this.normalizeKey(key));
215+
}
216+
}

test/client/factory.spec.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('ClientFactory', () => {
5050
preferredTransports: ['UnknownTransport'],
5151
};
5252
expect(() => new ClientFactory(options)).to.throw(
53-
'Unknown preferred transport: UnknownTransport, available transports: Transport1'
53+
'Unknown preferred transport: UnknownTransport, available transports: TRANSPORT1'
5454
);
5555
});
5656

@@ -71,6 +71,30 @@ describe('ClientFactory', () => {
7171

7272
expect(factory.options).to.equal(options);
7373
});
74+
75+
it('should accept preferred transport with different case', () => {
76+
const options: ClientFactoryOptions = {
77+
transports: [mockTransportFactory1],
78+
preferredTransports: ['transport1'], // lowercase, but Transport1 is registered
79+
};
80+
81+
// Should not throw
82+
const factory = new ClientFactory(options);
83+
84+
expect(factory.options).to.equal(options);
85+
});
86+
87+
it('should detect duplicate transports with different case as duplicates', () => {
88+
const transport1Lower = {
89+
protocolName: 'transport1', // lowercase
90+
create: vi.fn(),
91+
};
92+
const options: ClientFactoryOptions = {
93+
transports: [mockTransportFactory1, transport1Lower], // Transport1 and transport1
94+
};
95+
96+
expect(() => new ClientFactory(options)).to.throw('Duplicate protocol name: transport1');
97+
});
7498
});
7599

76100
describe('createClient', () => {
@@ -163,6 +187,46 @@ describe('ClientFactory', () => {
163187
expect(client.config).to.equal(clientConfig);
164188
});
165189

190+
it('should match transport with case-insensitive protocol name', async () => {
191+
// Transport factory uses "Transport1" but agent card uses "transport1" (lowercase)
192+
agentCard.preferredTransport = 'transport1';
193+
const factory = new ClientFactory({ transports: [mockTransportFactory1] });
194+
195+
const client = await factory.createFromAgentCard(agentCard);
196+
197+
expect(client).to.be.instanceOf(Client);
198+
expect(mockTransportFactory1.create).toHaveBeenCalledExactlyOnceWith(
199+
'http://transport1.com',
200+
agentCard
201+
);
202+
});
203+
204+
it('should match HTTP+JSON transport regardless of case', async () => {
205+
const httpJsonFactory = {
206+
protocolName: 'HTTP+JSON',
207+
create: vi.fn().mockResolvedValue(mockTransport),
208+
};
209+
agentCard.preferredTransport = 'http+json'; // lowercase
210+
const factory = new ClientFactory({ transports: [httpJsonFactory] });
211+
212+
await factory.createFromAgentCard(agentCard);
213+
214+
expect(httpJsonFactory.create).toHaveBeenCalledTimes(1);
215+
});
216+
217+
it('should match JSONRPC transport regardless of case', async () => {
218+
const jsonRpcFactory = {
219+
protocolName: 'JSONRPC',
220+
create: vi.fn().mockResolvedValue(mockTransport),
221+
};
222+
agentCard.preferredTransport = 'JsonRpc'; // mixed case
223+
const factory = new ClientFactory({ transports: [jsonRpcFactory] });
224+
225+
await factory.createFromAgentCard(agentCard);
226+
227+
expect(jsonRpcFactory.create).toHaveBeenCalledTimes(1);
228+
});
229+
166230
it('should use card resolver with default path', async () => {
167231
const cardResolver = {
168232
resolve: vi.fn().mockResolvedValue(agentCard),

0 commit comments

Comments
 (0)