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

Make it possible to return data from a raised intent #432

Closed
Tracked by #444
kriswest opened this issue Jul 26, 2021 · 20 comments · Fixed by #495
Closed
Tracked by #444

Make it possible to return data from a raised intent #432

kriswest opened this issue Jul 26, 2021 · 20 comments · Fixed by #495
Assignees
Labels
api FDC3 API Working Group channels feeds & transactions Channels, Feeds & Transactions Discussion Group enhancement New feature or request intents
Milestone

Comments

@kriswest
Copy link
Contributor

kriswest commented Jul 26, 2021

Output from the Channels, Feeds & Transactions discussion group: #420
Supercedes #201 & #372

Enhancement Request

It should be possible to return data from an application that is resolving a raised intent and for the application that raised that intent to receive it back. This was part of the original idea for intents (see https://fdc3.finos.org/docs/api/spec#raising-intents) , but was never correctly implemented in the spec such that it was possible to populate the data element in the IntentResolution data structure.

Use Case:

There are multiple existing use-cases that would benefit from being able to return data from a raised intent:

Other examples include:

  • raising an order (or other similar) ticket and receiving back a ticket number
  • performing a credit check and receiving back a response
  • retrieving a single price or another piece of market data from another application

Note: Channels are a poor fit for a single response:

  • Overhead for creating/retrieving a channel, which has to happen at both ends
  • You would probably need to return the name of the response channel, which is in-of-itself a use-case for a transaction
  • Possible synchronization issues (was the response already sent before I joined?)
@kriswest kriswest added enhancement New feature or request api FDC3 API Working Group intents channels feeds & transactions Channels, Feeds & Transactions Discussion Group labels Jul 26, 2021
@kriswest
Copy link
Contributor Author

kriswest commented Jul 26, 2021

Proposal to resolve:
The existing signature for raiseIntent and the returned data structure are largely sufficient already:

raiseIntent(intent: string, context: Context, app?: TargetApp): Promise<IntentResolution>;

interface IntentResolution {
  source: TargetApp;
  data?: Context; //deprecated in 1.2 - update type to return a `Context` rather than `object`
  version: string;
}

However, the addIntentListener function and handler function signature needs to be updated to permit data to be returned:

addIntentListener(intent: string, handler: IntentHandler): Listener; //currently based on a ContextHandler
type IntentHandler = (context: Context) => Promise<Context|void>     //permit an optional Context return type

The main change is that the desktop agent has to wait for the IntentHandler to return before returning the IntentResolution (which might be void or a context object to return in the intent resolution). Any timeouts should be handled the code raised the intent - if it no longer cares about a response it can discard the promise.

An alternative proposal is to allow the return of the IntentResolution immediately (on the selection of an app to resolve the intent, where necessary) by updating the resolution signature to:

interface IntentResolution {
  source: TargetApp;
  data: Promise<Context | void>; //Promise that should resolve when IntentHandler has resolved.
  version: string;
}

This is arguably superior as the app that raised the intent will know which application is resolving it while waiting on a response, whereas, if we wait for the IntentHandler's promise to resolve before returning we won't know which app we are waiting on. I.e. The return of the IntentResolution is only dependent on the DesktopAgent/resolver and not also on the application resolving the intent.

Finally, an extension to the AppD config for intents may also be advisable, to identify that the app returns a context for a particular intent:

{ 
    "appId": "string",
    "name": "string",
    ...
    "intents": [
        {
            "name": "string",
            "displayName": "string",
            "contexts": [ "string" ],
            "returns": "string",           //Context type name returned, e.g. "fdc3.order"
            "customConfig": { }
        }
    ]
}

@kriswest
Copy link
Contributor Author

Discussion group notes:

  • Later proposal preferred by several participants
    • Desktop agent can resolve IntentResolution promise as soon as the intent/context is delivered to resolving app
    • Then the app that raised intent can wait on the intenthandler to resolve the promise for the data (or void)
  • Several participants are nervous about returning data from intents, which they think of as 'fire and forget'.
  • Other participants think introducing a separate set of concepts that mirror intents, but allow the return of data would add unnecessary complexity to the standard
    • The proposed additions are optional - you do not need to wait on the promises

@mattjamieson to raise an alternative proposal for this issue for consideration

@mattjamieson
Copy link
Contributor

Prior art

In Android, raising a ViewContact intent might look like:

Intent viewChartIntent = new Intent(Intent.ACTION_VIEW, contractUri);
if (intent.resolveActivity(getPackageManager()) != null) { // Resolve the app which should handle this intent
    startActivity(intent);
}

SelectContact might be:

static final int REQUEST_SELECT_CONTACT = 1;

public void selectContact() {
    Intent intent = new Intent(Intent.ACTION_PICK);
    if (intent.resolveActivity(getPackageManager()) != null) {
        startActivityForResult(intent, REQUEST_SELECT_CONTACT);
    }
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
    if (requestCode == REQUEST_SELECT_CONTACT && resultCode == RESULT_OK) {
        Uri contactUri = data.getData();
        // Do something with the selected contact at contactUri
        ...
    }
}

FDC3 equivalent

  • data field in IntentResolution remains deprecated
  • IntentResolution promise has a single meaning: the target app was resolved and the intent was delivered
  • add a new raiseIntentForResult function with an additional resultHandler parameter
raiseIntentForResult(intent: string, context: Context, resultHandler: ContextHandler, app?: TargetApp): Promise<IntentResolution>;

addIntentListener(intent: string, handler: IntentHandler): Listener; //currently based on a ContextHandler
type IntentHandler = (context: Context) => Promise<Context|void>     //permit an optional Context return type

Comments

  • I'm not a fan of the parameter reordering (due to app being optional)
  • We could make resultHandler optional too, but if it was then maybe it could just be an optional parameter to the existing raiseIntent method
  • The DesktopAgent would have to take care of providing the trigger for the IntentResolution promise (as returning from IntentHandler would indicate the return of a result)

@Qiana-Citi
Copy link

@kriswest In our real use-cases, we do need returning data from intents.

Our real use-case, the order/trade booking app (A) raises intent to the regulation checking app (B), then B will resolve the intent (finish the regulation checking) and return the intent resolution (the checking result) to A, so that A can tell whether the order/trade is good to be processed.

In our current implementation of raiseIntent/addIntentListener, this is exact what we do to get IntentResolution data from apps that are resolving intents.

The main change is that the desktop agent has to wait for the IntentHandler to return before returning the IntentResolution (which might be void or a context object to return in the intent resolution). Any timeouts should be handled the code raised the intent - if it no longer cares about a response it can discard the promise.

Rather than the signature for listener, what we need is the enhancement on the signature for raiseIntent.

raiseIntent(intent: string, context: Context, app?: TargetApp): Promise<IntentResolution>;

Currently, the return of raiseIntent is promise of a single IntentResolution, which is fine for the case of raising intent for a single app. However, since TargetApp is optional, we can raise intent for multiple apps. In such case, how can a single IntentResolution handle with multiple apps’ returns?

@thorsent
Copy link
Contributor

@Qiana-Citi

Currently, the return of raiseIntent is promise of a single IntentResolution, which is fine for the case of raising intent for a single app. However, since TargetApp is optional, we can raise intent for multiple apps. In such case, how can a single IntentResolution handle with multiple apps’ returns?

The DesktopAgent will only ever deliver an intent to a single recipient. When there are multiple possible recipients, the desktop agent will typically prompt the end user to pick the recipient (i.e. it will "resolve" the intent).

Please also note that "TargetApp" defines an app type (name) - not an instance.

A call to raiseIntent will therefore only ever receive one IntentResolution.

The pattern you're describing is sometimes referred to as a "wrangler" pattern (a client retrieves multiple, collated responses from a set of services). I don't think there has been any discussion on implementing such a pattern. You could conceivably create such a pattern using context broadcasting on "App Channels".

@thorsent
Copy link
Contributor

Here is an iteration based on the suggestion that (in a two-stage resolution) returned data should resolve from a function that returns a promise rather than directly from a promise. I think that the resulting control flow using await statements is an improvement in readability.

// The promise returned from a call to raiseIntent() resolves when the desktop agent
// has delivered the intent to an intended app (e.g. the handler from addIntentListener()
// has been called on the intended app).
interface IntentResolution {
	readonly source: TargetApp;
	readonly version: string;
	// A client may then subsequently await on dataStream() to receive data from the intended app.
	// If the intended app does not resolve addIntentListener() with a promise, then it means that
	// no data is returned, and this promise will reject if called.
	getData : () => Promise<any>;
}

// The ContextHandler signature is revised slightly to accommodate optional data resolution.
// By returning a promise, the intent handler signals to the desktop agent that it will be returning data.
// If it does not return a promise, then it is "set and forget" (clients listening on getData() will reject)
type ContextHandler = (context: Context) => Promise<any>|void;



/**
 * Client Side Usage Example
 */

const resolution = await fdc3.raiseIntent("trade", { type: "tradeData", id : { symbol: "AAPL"}});

try{
	const tradeResult = await resolution.getData();
	console.log(`${resolution.source} returned ${tradeResult}`);
}catch(error){
	console.log(`${resolution.source} doesn't return data`);
}

/**
 * Intended App Usage Example: Case #1
 * The intended app will return data to the client who should be awaiting on getData()
 */

fdc3.addIntentListener("trade", (context) => {
	return new Promise<any>((resolve) => {
		// go place trade
		resolve({result: "trade placed successfully"});
	});
});

/**
 * Intended App Usage Example: Case #2
 * The intended app doesn't return data. The client will get an exception if they listen on getData();
 */

fdc3.addIntentListener("trade", (context) => {
	// go do something
	return;
});

@Qiana-Citi
Copy link

@thorsent

Please also note that "TargetApp" defines an app type (name) - not an instance.

Actually we do have real use cases that need TargetApp as an window instance. For the multi-instance applications, there could be multiple window instances running simultaneously. If the target could only be the app, which instance should resolve the raised intent? Or whether a new window instance should be launched and resolve the intent?

In our real use-case, the view holdings application is a multi-instance one. Usually users would open multiple ViewHolding windows with different views according to specific client, sector, product and etc. When an intent is raised (to view a specific instrument), users will choose which window/view to resolve this intent.

Even if the current standard only defined TargetApp as an app type(name), we would recommend to extend the scope of TargetApp to take care of the multi-instance applications.

@HansHuang
Copy link

@thorsent

Here is an iteration based on the suggestion that (in a two-stage resolution) returned data should resolve from a function that returns a promise rather than directly from a promise.

What's the benefit from "two-stage" resolution from exciting API schema ?

Two side effects I can think about:

  1. Obviously it breaks backward compatibility.
  2. It brings implementation complexity for both Desktop Agent & App sides.
    For every resolution, need to consider transaction scenario between 2 intents.

In our production cases, we did implement data return from intent since FDC3 v1.0, by the data field in IntentResolution interface. I don't know the story why this field is marked deprecated in latest standard.

One suggestion is for the data type definition as Context, idea is all data payload in FDC3 should be standardized:

interface IntentResolution {
  source: string;
  data?: Context ;
  version: string;
}

@HansHuang
Copy link

For TargetApp parameter in raiseIntent, actually both app type and instance are supported in our production.
But in 2 years practice of FDC3, we found no real case for "app type" when raiseing intent. It's one-to-many relationship for app & intents, but one intent always map to single app.

For example we have multiple trade booking apps for different user roles, products or markets, but for single user only have single trade booking app on single function. e.g. trader won't see sales' app; bond trade shouldn't go to derivatives trade booking app.

For raising intent to app instance, agree with @Qiana-Citi , it's much more common case.
For example user can have multiple TradeHistory instances on different view in his/her desktop. When raises intent from another app to "ViewTradeHistory", she/he only wants one of them response the intent to update data view, other instances should not be impacted -- that's the purpose of opening multiple instances. Corresponding there is a UI screen for user to choose which instance should be intent target and decision can be remembered for next time.

@thorsent
Copy link
Contributor

thorsent commented Sep 3, 2021

@HansHuang and @Qiana-Citi
Re: TargetApp, we might want a different github issue. Targeting instances sometimes comes up so it would be good to know more about your solution. The spec is pretty open about how a Desktop Agent can decide to resolve when there are two app instances. Usually it's pop-up UI and I think that remembering the user's choice is good UI - but could be up to each desktop agent to decide on UX. Typescript for TargetApp is currently string | AppMetaData. I don't think the "string" part is clearly defined but I think is usually interpreted "app type".

Re: two-stage. Yes, there is debate about two-stage. Everyone agrees it adds complexity. Here is the use case (AFAIK) that is a concern:

Blotter App - raiseIntent "Trade"
OMS App - (a) Receive intent, (b) dialog user to confirm trade, (c) execute trade, (d) return trade result

Blotter App wants to confirm for the end user when the OMS receives the trade and then (later) display the trade result. With single-stage, the user might wonder if their trade was handled.

(But, I think it's fair to argue that "two-phase" = "stream". Maybe this use case could be solved by feeds instead of intents).

@kriswest
Copy link
Contributor Author

kriswest commented Sep 6, 2021

@HansHuang @Qiana-Citi @thorsent
I can confirm that intents should only be delivered to a single application. See the specification section on Resolvers for details.

The string in the TargetApp type relates to the app name field for backwards compatibility with FDC3 1.1 and 1.0. Hence the following two calls are equivalent:

fdc3.raiseIntent("ViewChart", myContext, "someApp")
fdc3.raiseIntent("ViewChart", myContext, {name: "someApp"})

while since 1.2 you can be more specific by also passing a version or appId, e.g.:

fdc3.raiseIntent("ViewChart", myContext, {name: "someApp", version: "1.0.1"})
fdc3.raiseIntent("ViewChart", myContext, {appId: "someApp_v1.0.1"})

I agree that we need a separate issue for extending TargetApp to support targeting app instances AND discovery of those instances. A significant effort was already made in this area, however, I'm not personally a fan of that proposal and would prefer to raise a new issue, which I'll take as an action item.

As @thorsent comments, the reason for IntentResolution to contain a promise for data rather than the data itself is so that the DesktopAgent can return as soon as it's delivered the intent (which might be immediate if targeting a running instance or after starting the app, waiting for it to register an intent listener and then delivering the intent/context to that listener). There is no need for it to wait for the targetted application to also return, which might be delayed by user interaction, network or indeed might never return! Dealing with this also introduces complexity, the need for timeouts etc. etc. which can be eliminated by the Desktop agent instead of returning a promise for the data that can be waited on independently.

I don't agree that this two-step solution is the same as a stream as there is only a single response from the application that received the intent. Personally, I believe that to be a slightly different use-case that needs its own handling - and that both use-cases occur often enough to warrant support in the standard.

@pbaize
Copy link

pbaize commented Sep 9, 2021

I'm inclined to wait to add this to the spec until we have more concrete use cases (with context types). In the meantime, nothing prevents implementation of something like the following on top of the current spec:

function registerIntentWithResultHandler(tansactionType, handler) {
    fdc3.addIntentListener(`fdc3.IntentWithResult.${transactionType}`, async (context) => {
        const transactionChannel = await fdc3.getOrCreateChannel(context.id.channel);
        transactionChannel.addContextListener(context.id.payload, (payloadContext) => {
             const response = await handler(payloadContext); // Response is a context here
             transactionChannel.broacast({type: 'fdc3.IntentWithResult.Response', id: {
                 responseType: response.type
             }});
             transactionChannel.broadcast(response);
        })
    });
}

async function raiseIntentForResult(transactionType, context) {
    const id = utils.getGuid(); // Some sort of guid util
    const transactionChannel = await fdc3.getOrCreateChannel(id);
    transactionChannel.broadcast({type: 'fdc3.IntentWithResult.Payload', id: context});
    const responsePromise = new Promise(resolve => {
       transactionChannel.addContextListener('fdc3.IntentWithResult.Response', context => {
           transactionChannel.addContextListener(context.id.responseType, resolve);
       });
    });
    await fdc3.raiseIntent(`fdc3.IntentWithResult.${transactionType}`, {type: 'fdc3.IntentWithResult.handshake', id: {
        channel: id
    }});
    return responsePromise;
}

@kriswest
Copy link
Contributor Author

kriswest commented Sep 9, 2021

Output from #452

  • Noted that we've had responses from two participants about needing (and having implemented) data responses from intents, in use for a couple of years
  • Achieved consensus that the updated 'option 2' proposal is workable and appropriate (if not yet proven in use)
  • will go to PR and not back on agenda
  • update return type from Promise<any> to Promise<Context>
  • a separate issue should be raised to add support to the appD (and perhaps also to findIntent) to express what context types an app should return

@kriswest kriswest modified the milestones: 2.0-candidates, 2.0 Nov 1, 2021
@bertrand-s
Copy link
Contributor

From our side (Symphony), I also confirm that we see great value in having optional result data associated to intents.

Having said that, what annoys me most is that the current proposal keeps intents untyped - i.e. we need to rely on documentation to know that a given intent name (e.g. ViewQuote) requires a special context argument (e.g. Instrument) and returns a specific context type (e.g. Quote).

This may be not the purpose of this discussion thread (I saw @rikoe proposed to introduce generics to FDC3 APIs) - but this is indeed something that could be solved with generics. The case of intents is however a bit special as we need to associate a string (the intent name) with 2 types (input and output contexts). This can be easily done by transforming the intent name into an object, as shown in this example:

type TargetApp = string; // for the example sake - to allow to copy/paste this extract in a TS file

interface IntentId<InputContext, OutputContext = void> {
    // InputContext and OutputContext types are not used here, but TS is smart enough to associate them with this IntentId
    name: string;
}

interface IntentResolution<Context = void> {
    source: TargetApp;
    data?: Promise<Context>;
    version: string;
}

// helper function to create an Intent identifier object that binds a name with some Input and Output types
function intentId<InputContext, OutputContext = void>(intentName: string): IntentId<InputContext, OutputContext> {
    return {
        name: intentName
    };
}

// raiseIntent with intent types
async function raiseIntent<Input, Output = void>(intent: IntentId<Input, Output>, context?: Input): Promise<IntentResolution<Output>> {
    return {
        version: "1.0",
        source: "xyz",
        // + data as Promise (optional here)
    }
}

// example on a modified ViewQuote intent
interface Instrument {
    type: 'fdc3.instrument';
    name: string;
    id: {
        ticker: string;
        // etc.
    }
}

interface Quote {
    type: 'example.fdc3.quote';
    from: string;
    price: number;
    currency: string;
    instrument: Instrument;
}

// ViewQuote intent definition: create an object that binds the "ViewQuote" name with the Instrument and Quote interfaces
export const ViewQuoteIntent = intentId<Instrument, Quote>("ViewQuote");

// call example
async function callIntent() {
    // auto-completion / TS validation works on all following lines
    const result = await raiseIntent(ViewQuoteIntent, { type: "fdc3.instrument", name: "Microsoft", id: { ticker: "MSFT" } });

    const d = await result.data;
    return `${d.currency}${d.price}`;
}

@rikoe
Copy link
Contributor

rikoe commented Nov 4, 2021

Thanks @bertrand-s for the detailed example. I really like this actually - this would be similar to what we did with the target argument, where we eventually changed it from string to string | AppTarget.

It's a great backward compatible way to provide more metadata about intent types to the API.

Please feel free the FDC3 Channels, Feeds and Transactions discussion group later today (in about an hour, in fact), if you'd like to discuss this more (meeting details in the FINOS Calendar / #489).

I am also planning to create two issues today, for adding generics to the FDC3 typings, and to include information in the AppD about intents that have return types, which are related.

Feel free to create an issue for what you suggest above, I am hopeful we should be able to get this into 2.0.

@ggeorgievx
Copy link
Member

ggeorgievx commented Nov 4, 2021

Great suggestion, @bertrand-s ! There has been some discussion in the group around introducing Request/Response type methods using the Interop API Glue42 co-authored with the Plexus Deutsche Bank team. Here is a link to the API.

In today's FDC3 Channels, Feeds and Transactions discussion group I will quickly go over and present the Streaming capabilities of that API and we will discuss whether it would be a good addition to the FDC3 API.

Both the Request/Response and Streaming methods are typed using the bilateral FDC3 context types as strings (link).

The definitions of the standardised context types are part of the @finos/fdc3 npm package.

@kriswest
Copy link
Contributor Author

kriswest commented Nov 4, 2021

@bertrand-s Note that the appD does define the required context types for intents, which are to be used for resolution - however this is not reflected in the API, rather it is assumed that that data will be used by a resolver UI provided by the Desktop Agent, or the findIntent function (if the optional context object is passed to it). @riko is also raising a separate issue to add details of returned data to the appD application schema.

Whilst I'm not opposed to the use of generics here, I wonder whether they are adding anything for input context types if the context object is also passed (e.g. to raiseIntent/findIntent). Further, we can't rely on the generics too heavily as they don't exist in base javascript and won't necessarily provide a compile check that returned (output) data is of the correct type. Just a word of caution - I am a fan of generics.

This could/should be raised as a separate issue - it could form part of the generic issue you are raising @rikoe.

Finally, @rikoe, some slight tweakage to the proposal is needed for it to be fully backwards compatible? The input type for the intent on raiseIntent has changed from string to a typed object (so well need an overload). Also the context input should not be optional.

@kriswest
Copy link
Contributor Author

kriswest commented Nov 4, 2021

P.S.

i.e. we need to rely on documentation to know that a given intent name (e.g. ViewQuote) requires a special context argument (e.g. Instrument) and returns a specific context type (e.g. Quote).

The Intents documentation only provides a list of 'possible contexts'. It is currently only the AppD application records that define which contexts and app will accept for an intent. This can vary from app-to-app, hence, documentation should not be relied upon.

If you need to know which apps can resolve an intent use fdc3.findIntent to programmatically access the resolver. You can pass an optional context object to filter down to only apps that will accept that context for the specified intent.

@thorsent
Copy link
Contributor

thorsent commented Nov 4, 2021

+1 for Typescript generics!

@kriswest
Copy link
Contributor Author

kriswest commented Nov 4, 2021

Perhaps we need to also propose optional return type arguments for raiseIntent and findIntent - generics will NOT provide the relevant info through to a resolver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group channels feeds & transactions Channels, Feeds & Transactions Discussion Group enhancement New feature or request intents
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants