Context Clearing #1379
Context Clearing #1379
Conversation
✅ Deploy Preview for fdc3 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -36,6 +36,7 @@ interface Channel { | |||
| broadcast(context: Context): Promise<void>; | |||
| getCurrentContext(contextType?: string): Promise<Context|null>; | |||
| addContextListener(contextType: string | null, handler: ContextHandler): Promise<Listener>; | |||
| clearContext(contextType?: string): Promise<void>; | |||
There was a problem hiding this comment.
I'm wondering if we should require an explicit null argument (contextType: string | null) to clear all types - which would match addContectListener or stick with this (matching getCurrentContext). Either works of course.
There was a problem hiding this comment.
To me it makes sense to keep it closer to getCurrentContext, because they seem most related to me, but happy to hear any thoughts/suggestions.
There was a problem hiding this comment.
There is a historical reason for addContextListener having an explicit null argument, the contextType argument was originally optional so the handler argument could be either first or second (when a type was specified). This was NOT good in practice so we moved to an explicit null argument and deprecated the single argument override:
https://fdc3.finos.org/docs/2.0/api/ref/DesktopAgent#addcontextlistener-deprecated
We don't have that problem here, so I agree its fine as is (matching getCurrentContext).
There was a problem hiding this comment.
Many thanks for taking this on @kemerava.
Could you add an entry to the CHANGLOG.md file please.
I think we should keep this open until after fdc3 for the web (#1191) is merged, when we will need to add schemas for a request and response message. The same may be needed in the bridging (the fdc3.nothing ewill propagate through normal broadcast messages, but I think this will need a specific message to indicate it happened through the clearContextAPI call).
Co-authored-by: Kris West <[email protected]>
There was a problem hiding this comment.
LGTM - although we could do with an example of listening for cleared context somewhere.
Holding approval until after fdc3-for-web is merged as we'll then need to add schemas to support it there. Bridging schemas are also needed, those should be added before this is merged.
Many thanks for the contribution @kemerava, much appreciated.
|
@kriswest could you please take a look at the conformance tests that I added? Thanks |
| @@ -52,3 +52,6 @@ | |||
| - `ACContextHistoryTyped`: Perform above test. | |||
| - `ACContextHistoryMultiple`: **B** Broadcasts multiple history items of both types. Ensure that only the last version of each type is received by **A**. | |||
| - `ACContextHistoryLast`: In step 5. **A** retrieves the _untyped_ current context of the channel via `const currentContext = await testChannel.getCurrentContext()`. Ensure that A receives only the very last broadcast context item _of any type_. | |||
| - `ACClearHistorySpecificContext`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.instrument')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and context is received successfully for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |||
There was a problem hiding this comment.
| - `ACClearHistorySpecificContext`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.instrument')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and context is received successfully for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |
| - `ACClearHistorySpecificContext`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.instrument')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and context is received successfully for `const contact = await testChannel.getCurrentContext('fdc3.contact')` |
| @@ -52,3 +52,6 @@ | |||
| - `ACContextHistoryTyped`: Perform above test. | |||
| - `ACContextHistoryMultiple`: **B** Broadcasts multiple history items of both types. Ensure that only the last version of each type is received by **A**. | |||
| - `ACContextHistoryLast`: In step 5. **A** retrieves the _untyped_ current context of the channel via `const currentContext = await testChannel.getCurrentContext()`. Ensure that A receives only the very last broadcast context item _of any type_. | |||
| - `ACClearHistorySpecificContext`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.instrument')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and context is received successfully for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |||
| - `ACClearHistoryAllContexts`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.nothing')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |||
There was a problem hiding this comment.
| - `ACClearHistoryAllContexts`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.nothing')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |
| - `ACClearHistoryAllContexts`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.nothing')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and for `const contact = await testChannel.getCurrentContext('fdc3.contact')` |
| @@ -52,3 +52,6 @@ | |||
| - `ACContextHistoryTyped`: Perform above test. | |||
| - `ACContextHistoryMultiple`: **B** Broadcasts multiple history items of both types. Ensure that only the last version of each type is received by **A**. | |||
| - `ACContextHistoryLast`: In step 5. **A** retrieves the _untyped_ current context of the channel via `const currentContext = await testChannel.getCurrentContext()`. Ensure that A receives only the very last broadcast context item _of any type_. | |||
| - `ACClearHistorySpecificContext`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.instrument')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and context is received successfully for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |||
| - `ACClearHistoryAllContexts`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.nothing')`. Ensure that in step 5 nothing is received for `const instrument = await testChannel.getCurrentContext('fdc3.instrument')` and for `const instrument = await testChannel.getCurrentContext('fdc3.contact')` | |||
| - `ACClearHistoryAllContextsSubscribedToNothing`: Perform the above test, except after step 3 clear context with `await testChannel.clearContext('fdc3.nothing')` and in step 4 call `await testChannel.addContextListener("fdc3.nothing", handler)` instead. Ensure that after clearing context, the handler is called. | |||
There was a problem hiding this comment.
A few thoughts on this case:
- I think you want to clear fdc3.instrument or all types (null) rather than fdc3.nothing
- As written, the nothing handler is added after calling clear, in that case it won't receive anything on an app channel. The handler needs to be in place before the clear.
- I think this needs to be 2 cases, one for clearing all types and another for a specific type that confirms the other is not affected and that the fdc3.nothing quotes the correct type.
- Finally, I think we need at least one case that checks the behavior of
userchannele- you'll work with the channel as an app channel to clear it (either retrrieving it by name, viagetUserChannels, orgetCUrrentChannel) but we should also confirm that it gets the fdc3.nothing broadcast and that it doesn't retain the cleared context.
Closes #1197
Adding context cleaning
@michael-bowen-sc