Skip to content

Commit db2cbf2

Browse files
authored
Merge pull request #1858 from finos/1857-starting-an-app-on-a-channel
Ensure current user channel is resolved before DesktopAgentProxy is used
2 parents 0cf94c3 + 4753fa2 commit db2cbf2

12 files changed

Lines changed: 151 additions & 81 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
6767
* Stopped fdc3-workbench flagging FDC3 version 2.2 as unsupported. ([#1841](https://github.com/finos/FDC3/pull/1841))
6868
* Resolved vulnerable dependencies (esbuild, serialize-javascript, elliptic) and consolidated shared devDependencies to simplify future maintenance. ([#1841](https://github.com/finos/FDC3/pull/1841))
6969
* Fixed the lack of handling of WCP6Disconnect messages in MessagePort example in the FDC3 Web reference implementation. ([#1854](https://github.com/finos/FDC3/pull/1854))
70+
* Fixed handling of DesktopAgents that start apps joined to a user channel by the agent-proxy by retrieving the current user channel on start-up. ([#1858](https://github.com/finos/FDC3/pull/1858))
71+
* Fixed a race condition in `DefaultChannelSupport` initialization where the current user channel was retrieved asynchronously without being awaited, which could cause apps started on a user channel by the Desktop Agent to miss their initial channel assignment. ([#1858](https://github.com/finos/FDC3/pull/1858))
7072

7173
## [FDC3 Standard 2.2](https://github.com/finos/FDC3/compare/v2.1..v2.2) - 2025-03-12
7274

packages/fdc3-agent-proxy/src/channels/ChannelSupport.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1-
import { Channel, ContextHandler, EventHandler, FDC3EventTypes, Listener, PrivateChannel } from '@finos/fdc3-standard';
1+
import {
2+
Channel,
3+
Connectable,
4+
ContextHandler,
5+
EventHandler,
6+
FDC3EventTypes,
7+
Listener,
8+
PrivateChannel,
9+
} from '@finos/fdc3-standard';
210

3-
export interface ChannelSupport {
11+
export interface ChannelSupport extends Connectable {
412
getUserChannel(): Promise<Channel | null>;
513
getUserChannels(): Promise<Channel[]>;
614
getOrCreate(id: string): Promise<Channel>;

packages/fdc3-agent-proxy/src/channels/DefaultChannelSupport.ts

Lines changed: 73 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
Channel,
3+
Connectable,
34
ContextHandler,
45
Listener,
56
PrivateChannel,
@@ -35,7 +36,7 @@ import {
3536
import { throwIfUndefined } from '../util/throwIfUndefined.js';
3637
import { Logger } from '../util/Logger.js';
3738

38-
export class DefaultChannelSupport implements ChannelSupport {
39+
export class DefaultChannelSupport implements ChannelSupport, Connectable {
3940
readonly messaging: Messaging;
4041
readonly channelSelector: ChannelSelector;
4142
readonly messageExchangeTimeout: number;
@@ -55,8 +56,14 @@ export class DefaultChannelSupport implements ChannelSupport {
5556
this.joinUserChannel(channelId);
5657
}
5758
});
59+
}
60+
61+
async connect(): Promise<void> {
62+
//retrieve the current user channel in case the Desktop Agent started us on a channel
63+
this.currentChannel = await this.getUserChannel();
5864

59-
this.addEventListener(async (e: ApiEvent) => {
65+
//register for channelChangedEvents to track any DesktopAgent managed user channel changes
66+
await this.addEventListener(async (e: ApiEvent) => {
6067
const cce = e as FDC3ChannelChangedEvent;
6168
const newChannelId = cce.details.currentChannelId;
6269
Logger.debug('Desktop Agent reports channel changed: ', newChannelId);
@@ -65,12 +72,12 @@ export class DefaultChannelSupport implements ChannelSupport {
6572

6673
// if theres a newChannelId, retrieve details of the channel
6774
if (newChannelId != null) {
68-
theChannel = (await this.getUserChannelsCached()).find(uc => uc.id == newChannelId) ?? null;
75+
theChannel = (await this.getUserChannels()).find(uc => uc.id == newChannelId) ?? null;
6976
if (!theChannel) {
7077
// Channel not found - query user channels in case they have changed for some reason
7178
Logger.debug('Unknown user channel, querying Desktop Agent for updated user channels: ', newChannelId);
7279
await this.getUserChannels();
73-
theChannel = (await this.getUserChannelsCached()).find(uc => uc.id == newChannelId) ?? null;
80+
theChannel = (await this.getUserChannels()).find(uc => uc.id == newChannelId) ?? null;
7481
if (!theChannel) {
7582
Logger.warn(
7683
'Received user channel update with unknown user channel (user channel listeners will not work): ',
@@ -81,83 +88,88 @@ export class DefaultChannelSupport implements ChannelSupport {
8188
}
8289

8390
this.currentChannel = theChannel;
84-
this.channelSelector.updateChannel(theChannel?.id ?? null, await this.getUserChannelsCached());
91+
this.channelSelector.updateChannel(theChannel?.id ?? null, await this.getUserChannels());
8592
}, 'userChannelChanged');
8693
}
8794

95+
async disconnect(): Promise<void> {
96+
// no-op
97+
}
98+
8899
async addEventListener(handler: EventHandler, type: FDC3EventTypes | null): Promise<Listener> {
89100
const listener = new DesktopAgentEventListener(this.messaging, this.messageExchangeTimeout, type, handler);
90101
await listener.register();
91102
return listener;
92103
}
93104

94105
async getUserChannel(): Promise<Channel | null> {
95-
const request: GetCurrentChannelRequest = {
96-
meta: this.messaging.createMeta(),
97-
type: 'getCurrentChannelRequest',
98-
payload: {},
99-
};
100-
const response = await this.messaging.exchange<GetCurrentChannelResponse>(
101-
request,
102-
'getCurrentChannelResponse',
103-
this.messageExchangeTimeout
104-
);
105-
106-
throwIfUndefined(
107-
response.payload.channel,
108-
'Invalid response from Desktop Agent to getCurrentChannel (channel should be explicitly null if no channel is set)!',
109-
response,
110-
ChannelError.NoChannelFound
111-
);
106+
if (this.currentChannel) {
107+
//if the current channel is know,, return it as this variable is maintained by a channelChangedEvent listener
108+
return this.currentChannel;
109+
} else {
110+
const request: GetCurrentChannelRequest = {
111+
meta: this.messaging.createMeta(),
112+
type: 'getCurrentChannelRequest',
113+
payload: {},
114+
};
115+
const response = await this.messaging.exchange<GetCurrentChannelResponse>(
116+
request,
117+
'getCurrentChannelResponse',
118+
this.messageExchangeTimeout
119+
);
112120

113-
//handle successful responses - errors will already have been thrown by exchange above
114-
/* istanbul ignore else */
115-
if (response.payload.channel) {
116-
return new DefaultChannel(
117-
this.messaging,
118-
this.messageExchangeTimeout,
119-
response.payload.channel.id,
120-
'user',
121-
response.payload.channel.displayMetadata
121+
throwIfUndefined(
122+
response.payload.channel,
123+
'Invalid response from Desktop Agent to getCurrentChannel (channel should be explicitly null if no channel is set)!',
124+
response,
125+
ChannelError.NoChannelFound
122126
);
123-
} else if (response.payload.channel === null) {
124-
//this is a valid response if no channel is set
125-
return null;
126-
} else {
127-
//Should not reach here as we will throw in exchange or throwIfNotFound
128-
return null;
127+
128+
//handle successful responses - errors will already have been thrown by exchange above
129+
/* istanbul ignore else */
130+
if (response.payload.channel) {
131+
return new DefaultChannel(
132+
this.messaging,
133+
this.messageExchangeTimeout,
134+
response.payload.channel.id,
135+
'user',
136+
response.payload.channel.displayMetadata
137+
);
138+
} else if (response.payload.channel === null) {
139+
//this is a valid response if no channel is set
140+
return null;
141+
} else {
142+
//Should not reach here as we will throw in exchange or throwIfNotFound
143+
return null;
144+
}
129145
}
130146
}
131147

132-
async getUserChannelsCached(): Promise<Channel[]> {
148+
async getUserChannels(): Promise<Channel[]> {
149+
//If the user channels are known, return them as they are not expected to change
133150
if (this.userChannels) {
134151
return this.userChannels;
135152
} else {
136-
this.userChannels = await this.getUserChannels();
153+
const request: GetUserChannelsRequest = {
154+
meta: this.messaging.createMeta(),
155+
type: 'getUserChannelsRequest',
156+
payload: {},
157+
};
158+
const response = await this.messaging.exchange<GetUserChannelsResponse>(
159+
request,
160+
'getUserChannelsResponse',
161+
this.messageExchangeTimeout
162+
);
163+
164+
//handle successful responses
165+
const channels = response.payload.userChannels!;
166+
this.userChannels = channels.map(
167+
c => new DefaultChannel(this.messaging, this.messageExchangeTimeout, c.id, 'user', c.displayMetadata)
168+
);
137169
return this.userChannels;
138170
}
139171
}
140172

141-
async getUserChannels(): Promise<Channel[]> {
142-
const request: GetUserChannelsRequest = {
143-
meta: this.messaging.createMeta(),
144-
type: 'getUserChannelsRequest',
145-
payload: {},
146-
};
147-
const response = await this.messaging.exchange<GetUserChannelsResponse>(
148-
request,
149-
'getUserChannelsResponse',
150-
this.messageExchangeTimeout
151-
);
152-
153-
//handle successful responses
154-
const channels = response.payload.userChannels!;
155-
this.userChannels = channels.map(
156-
c => new DefaultChannel(this.messaging, this.messageExchangeTimeout, c.id, 'user', c.displayMetadata)
157-
);
158-
return this.userChannels;
159-
}
160-
161173
async getOrCreate(id: string): Promise<Channel> {
162174
const request: GetOrCreateChannelRequest = {
163175
meta: this.messaging.createMeta(),
@@ -223,7 +235,7 @@ export class DefaultChannelSupport implements ChannelSupport {
223235
this.messageExchangeTimeout
224236
);
225237
this.currentChannel = null;
226-
this.channelSelector.updateChannel(null, await this.getUserChannelsCached());
238+
this.channelSelector.updateChannel(null, await this.getUserChannels());
227239
}
228240

229241
async joinUserChannel(id: string) {
@@ -240,7 +252,7 @@ export class DefaultChannelSupport implements ChannelSupport {
240252
this.messageExchangeTimeout
241253
);
242254

243-
const userChannels = await this.getUserChannelsCached();
255+
const userChannels = await this.getUserChannels();
244256
this.currentChannel = userChannels.find(c => c.id == id) ?? null;
245257
if (this.currentChannel == null) {
246258
throw new Error(ChannelError.NoChannelFound);

packages/fdc3-agent-proxy/test/features/broadcast.feature

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ Feature: Broadcasting
2828
| payload.channelId | payload.context.type | payload.context.name | matches_type |
2929
| one | {null} | {null} | joinUserChannelRequest |
3030
| {null} | {null} | {null} | getUserChannelsRequest |
31-
| {null} | {null} | {null} | getCurrentChannelRequest |
3231
| one | fdc3.instrument | Apple | broadcastRequest |
3332

3433
Scenario: Context listener receives originating app metadata

packages/fdc3-agent-proxy/test/features/user-channel-sync.feature

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ Feature: Updating User Channel State
2727
| one | {null} | {null} | joinUserChannelRequest |
2828
| {null} | {null} | {null} | getUserChannelsRequest |
2929
| one | fdc3.instrument | {null} | getCurrentContextRequest |
30-
| {null} | {null} | {null} | getCurrentChannelRequest |
3130
| one | fdc3.instrument | {null} | getCurrentContextRequest |
3231

3332
Scenario: Changing User Channel Doesn't Receive Incorrect Context on Listener
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
Feature: User Channels Support where the Desktop Agent puts the app on a channel
2+
3+
Background: Desktop Agent API
4+
Given User Channels one, two and three
5+
Given schemas loaded
6+
Given A Desktop Agent in "api" that puts apps on channel "one"
7+
Given "instrumentMessageOne" is a BroadcastEvent message on channel "one" with context "fdc3.instrument"
8+
9+
Scenario: Initial User Channel
10+
At startup, the user channel should be set
11+
12+
When I call "{api}" with "getCurrentChannel"
13+
Then "{result}" is an object with the following contents
14+
| id | type | displayMetadata.color |
15+
| one | user | red |
16+
17+
Scenario: Adding a Typed Listener on a given User Channel
18+
Given "resultHandler" pipes context to "contexts"
19+
And I call "{api}" with "addContextListener" with parameters "fdc3.instrument" and "{resultHandler}"
20+
And messaging receives "{instrumentMessageOne}"
21+
Then "{contexts}" is an array of objects with the following contents
22+
| id.ticker | type | name |
23+
| AAPL | fdc3.instrument | Apple |
24+
And messaging will have posts
25+
| payload.channelId | payload.contextType | matches_type |
26+
| {null} | fdc3.instrument | addContextListenerRequest |
27+
| one | fdc3.instrument | getCurrentContextRequest |
28+
29+
Scenario: I should be able to leave a user channel, and not receive messages on it
30+
Given "resultHandler" pipes context to "contexts"
31+
And I call "{api}" with "addContextListener" with parameters "fdc3.instrument" and "{resultHandler}"
32+
And I call "{api}" with "leaveCurrentChannel"
33+
Then messaging will have posts
34+
| payload.channelId | payload.contextType | payload.listenerUUID | matches_type |
35+
| {null} | fdc3.instrument | {null} | addContextListenerRequest |
36+
| one | fdc3.instrument | {null} | getCurrentContextRequest |
37+
| {null} | {null} | {null} | leaveCurrentChannelRequest |
38+
| {null} | {null} | {null} | getUserChannelsRequest |
39+
And messaging receives "{instrumentMessageOne}"
40+
Then "{contexts}" is an array of objects with the following contents
41+
| id.ticker | type | name |

packages/fdc3-agent-proxy/test/features/user-channels.feature

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ Feature: Basic User Channels Support
6060
| payload.channelId | matches_type |
6161
| one | joinUserChannelRequest |
6262
| {null} | getUserChannelsRequest |
63-
| {null} | getCurrentChannelRequest |
6463

6564
Scenario: Changing Channel via Deprecated API
6665
You should be able to join a channel knowing it's ID.
@@ -74,7 +73,6 @@ Feature: Basic User Channels Support
7473
| payload.channelId | matches_type |
7574
| one | joinUserChannelRequest |
7675
| {null} | getUserChannelsRequest |
77-
| {null} | getCurrentChannelRequest |
7876

7977
Scenario: Adding a Typed Listener on a given User Channel
8078
Given "resultHandler" pipes context to "contexts"
@@ -192,8 +190,6 @@ Feature: Basic User Channels Support
192190
| payload.channelId | payload.context.type | payload.context.id.ticker | matches_type |
193191
| one | {null} | {null} | joinUserChannelRequest |
194192
| {null} | {null} | {null} | getUserChannelsRequest |
195-
| {null} | {null} | {null} | getCurrentChannelRequest |
196-
| {null} | {null} | {null} | getCurrentChannelRequest |
197193
| one | fdc3.instrument | AAPL | broadcastRequest |
198194
| one | {null} | {null} | getCurrentContextRequest |
199195

@@ -279,9 +275,6 @@ Feature: Basic User Channels Support
279275
| one | user | red | triangle | The one channel |
280276
| two | user | red | triangle | The two channel |
281277
| three | user | red | triangle | The three channel |
282-
And messaging will have posts
283-
| meta.source.appId | meta.source.instanceId | matches_type |
284-
| cucumber-app | cucumber-instance | getUserChannelsRequest |
285278

286279
Scenario: Destructured joinUserChannel and getCurrentChannel work correctly
287280
When I destructure method "joinUserChannel" from "{api}"
@@ -295,7 +288,6 @@ Feature: Basic User Channels Support
295288
| payload.channelId | matches_type |
296289
| one | joinUserChannelRequest |
297290
| {null} | getUserChannelsRequest |
298-
| {null} | getCurrentChannelRequest |
299291

300292
Scenario: Destructured channel getCurrentContext after broadcast
301293
Given "resultHandler" pipes context to "contexts"

packages/fdc3-agent-proxy/test/step-definitions/generic.steps.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ const logLevel = LogLevel.WARN;
2020
const schemaBasePath = path.join(import.meta.dirname, '../../../');
2121
setupGenericSteps(schemaBasePath);
2222

23-
Given('A Desktop Agent in {string}', async (world: CustomWorld, field: string) => {
23+
function createDesktopAgent(world: CustomWorld, field: string, initialChannelId?: string) {
2424
if (!world.messaging) {
25-
world.messaging = new TestMessaging(world.props[CHANNEL_STATE]);
25+
world.messaging = new TestMessaging(world.props[CHANNEL_STATE], initialChannelId);
2626
}
2727

2828
// n.b. using short timeouts to avoid extending tests unnecessarily
@@ -31,13 +31,29 @@ Given('A Desktop Agent in {string}', async (world: CustomWorld, field: string) =
3131
const is = new DefaultIntentSupport(world.messaging, new SimpleIntentResolver(world), 1500, 3000);
3232
const as = new DefaultAppSupport(world.messaging, 1500, 3000);
3333

34-
const da = new DesktopAgentProxy(hs, cs, is, as, [hs], logLevel);
34+
const da = new DesktopAgentProxy(hs, cs, is, as, [hs, cs], logLevel);
35+
return da;
36+
}
37+
38+
Given('A Desktop Agent in {string}', async (world: CustomWorld, field: string) => {
39+
const da = createDesktopAgent(world, field);
3540
await da.connect();
3641

3742
world.props[field] = da;
3843
world.props['result'] = null;
3944
});
4045

46+
Given(
47+
'A Desktop Agent in {string} that puts apps on channel {string}',
48+
async (world: CustomWorld, field: string, channelId: string) => {
49+
const da = createDesktopAgent(world, field, channelId);
50+
await da.connect();
51+
52+
world.props[field] = da;
53+
world.props['result'] = null;
54+
}
55+
);
56+
4157
When('messaging receives a heartbeat event', (world: CustomWorld) => {
4258
world.messaging?.receive({
4359
type: 'heartbeatEvent',

packages/fdc3-agent-proxy/test/support/TestMessaging.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export class TestMessaging extends AbstractMessaging {
111111

112112
readonly automaticResponses: AutomaticResponse[];
113113

114-
constructor(channelState: { [key: string]: Context[] }) {
114+
constructor(channelState: { [key: string]: Context[] }, initialChannelId?: string) {
115115
super({ appId: 'cucumber-app', instanceId: 'cucumber-instance' });
116116

117117
this.channelState = channelState;
@@ -126,7 +126,7 @@ export class TestMessaging extends AbstractMessaging {
126126
new FindInstances(),
127127
new Open(),
128128
new GetOrCreateChannel(),
129-
new ChannelState(this.channelState),
129+
new ChannelState(this.channelState, initialChannelId),
130130
new GetUserChannels(),
131131
new RegisterListeners(),
132132
new UnsubscribeListeners(),

0 commit comments

Comments
 (0)