Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API in 2.0 for retrieving App MetaData from an instanceIdentifier #734

Closed
nkolba opened this issue May 27, 2022 · 10 comments · Fixed by #751
Closed

Add API in 2.0 for retrieving App MetaData from an instanceIdentifier #734

nkolba opened this issue May 27, 2022 · 10 comments · Fixed by #751
Labels
api FDC3 API Working Group enhancement New feature or request
Milestone

Comments

@nkolba
Copy link
Contributor

nkolba commented May 27, 2022

Enhancement Request

Use Case:

As an application developer, I want to be able to retrieve the metadata of an application I'm inter-oping with on demand.

Additional Information

Proposed API:

getAppMetadata(appId : AppIdentifier) : Promise<AppMetadata>;


const appMetadata = await fdc3.getAppMetadata({
         appId: theAppId;
         instanceId: theInstanceId
});

Note: to be consistent with other APIs, this takes an argument of AppIdentifier - which mean an instanceId is optional, which is a bit misaligned with the use case of using this API to get the Metadata of an instance you are interacting with. The other option would be to change the signature to getAppMetadata(instanceId : string) - which maybe is not as consistent with the other ways we're accessing this kind of metadata.

@nkolba nkolba added enhancement New feature or request api FDC3 API Working Group labels May 27, 2022
@kriswest
Copy link
Contributor

Note: to be consistent with other APIs, this takes an argument of AppIdentifier - which mean an instanceId is optional, which is a bit misaligned with the use case of using this API to get the Metadata of an instance you are interacting with.

I think this is a reasonable way to do things as at present you will never receive an instanceId without an appId
(as part of an AppMetadata returned by other API calls). You may, however, receive an appId without an instanceId (e.g. from fdc3.findIntent or from outside the scope of the desktop agent) and retrieving metadata for such is another use case.

However, at present, other API calls that deal with appId all (may) return a full AppMetadata. Adding the proposed API may encourage some divergence in implementations as less than the full AppMetadata could be returned and then the rest retrieved via this API. Hence, if the intention is to improve developer ergonomics then you might also need to reduce some functions to returning AppIdentifier. The likely candidates are ContextMetadata (passed to ContextHandler and IntentHandler), fdc3.open, fdc3.findInstances and the IntentResolution returned from fdc3.raiseIntent and fdc3.raiseIntentForContext - but not fdc3.findIntent and fdc3.findIntentForContext whose use cases always demand the retrieval of the full AppMetadata.

Hence, I think we should also consider whether it is better for App developers (rather than Platform developers) for a desktop agent to always return the AppMetadata object and to upgrade some of the fields to required (perhaps just title) as this would negate the need to ever manually retrieve the AppMetadata...

Finally, if this is to be added to the 2.0 release, a PR will need to be raised and voted in by email before the Thursday 23rd June meeting. Otherwise, it'll need to go into the 2.1-candidates milestone.

@nkolba
Copy link
Contributor Author

nkolba commented May 27, 2022

I'd be interested in other opinions on this before putting in a PR for 2.0.

Yes. it would be preferable if ContextMetadata and IntentResolution only return AppIdentifier and then AppMetadata was retrieved as needed via the API. I think it's 6 one way 1/2 dozen another in terms of dev ergonomics. While an additional call needs to be made in some cases, it also means that app devs won't have to wrangle with other cases where they need to retrieve the metadata after the fact of a Context or Intent event.

The benefit of doing it this way of course is that we are not building into the design the need to attach potentially large and mostly unneeded data to almost every message in the system. What happens if someone wants to put base64 encoded images in their AppMetadata (for example)?

@kriswest
Copy link
Contributor

I think it's 6 one way 1/2 dozen another in terms of dev ergonomics.
Agreed. Its either got to be all one way or the other, the middle ground is where we're going to end up with variation between desktop agents, resulting in greater complexity for app developers to deal with and more bugs/need for testing on multiple desktop agents.

What happens if someone wants to put base64 encoded images in their AppMetadata (for example)?

oof, data URLs. Never seen that done in an AppD, but suppose it could be. However, there's no guarantee it'd be supported by a particular desktop agent implementation so its likely a bad idea.

That made me notice that AppMetadata is slightly further out-of-sync with the AppD record than before (e.g. images vs. screenshots, lang and localizedVersions etc.). Perhaps AppMetadata should really be a pass-through of the AppD record and have everything else use AppIdentifier?

@nkolba
Copy link
Contributor Author

nkolba commented May 27, 2022

Perhaps AppMetadata should really be a pass-through of the AppD record and have everything else use AppIdentifier?

yes!

@kriswest
Copy link
Contributor

kriswest commented Jun 1, 2022

From a colleague @ Cosaic:

I like the approach. AppIdentifier is what’s needed in most calls, and meanwhile it will be simpler for AppMetadata = AppD record (satisfying another outstanding request for a method to retrieve that info). It all seems worth the cost of an additional API function.

This would make security/privacy/filtering of AppD information a desktop agent concern.

@robmoffat
Copy link
Member

Regarding AppMetadata == AppD Record.

Wouldn't this be a breaking change? This might not be possible for 2.1 if so, right?

@kriswest
Copy link
Contributor

kriswest commented Jun 7, 2022

@robmoffat

Regarding AppMetadata == AppD Record.

Wouldn't this be a breaking change? This might not be possible for 2.1 if so, right?

Actually, I think that would mostly be an additive change as the appD fields are (I believe) a superset of the current AppMetadata fields (except for instanceId of course). This was a deliberate choice in earlier versions. See:

However, I think adding this API (and the implied change to reduce other API calls to returning AppIdentifier) is actually a breaking change (although less obviously so). It changes the API calls you might make by adding in calls to retrieve AppMetadata to supply titles, icons etc..

That said, I think it might be the right change to make.

@kriswest
Copy link
Contributor

The maintainers met on this issue and would like to resolve it in 2.0. Hence a PR has been raised (#751) and has been passed to the maintainers for review. Hopefully, it can be pre-agreed and then passed to the Standards Working Group via email for approval before the 2.0 predraft is put to a vote (on June 23rd).

@robmoffat
Copy link
Member

...and there was much rejoicing.

Have you added a tag?

@kriswest
Copy link
Contributor

Yes https://github.com/finos/FDC3/releases/tag/v2.0
But not yet a release (as there as ton of content to write for that and its not the 'active' release for 45 days after adoption)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants