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

Change Request: Extra properties for Participant #114

Open
tw-aungthawaye opened this issue Aug 15, 2022 · 14 comments
Open

Change Request: Extra properties for Participant #114

tw-aungthawaye opened this issue Aug 15, 2022 · 14 comments
Assignees
Labels
admin-api-change-request A change request made to the Admin API

Comments

@tw-aungthawaye
Copy link

Open API for FSP Interoperability - Change Request

Table of Contents

1. Preface

___

This section contains basic information regarding the change request.

1.1 Change Request Information

| Requested By | Aung Thaw Aye |
| Change Request Status | In review ☒ / Approved ☐ / Rejected ☐ |
| Approved/Rejected Date | |

1.2 Document Version Information

Version Date Author Change Description
1.0 2022-08-15 Aung Thaw Aye To allow participant to have extra properties.

2. Problem Description

___

2.1 Background

We have received a feedback from DFSPs regarding the result reports. DFSPs would like to see the liquidity account info at their reports. At this moment, there are three types of reports ("DFSP settlement statement report", “Settlement Initiation Report” and “Settlement Initiation Upload Report”) sent to them as in the following pictures :

DFSP Settlement Statement Report :

DFSP Settlement

Settlement Initiation Report :

Settlement Initiation Report

Settlement Initiation Upload Report :

Settlement Initiation Upload Report

These highlighted areas are “liquidity account information” (the bank ID) that the DFSPs requested.

2.2 Current Behaviour

In current reporting, we need to hard-code this liquidity account information in JSON inside the code of the report generation, and display to DFSPs. But this interim solution is not suitable for most of the cases because whenever there is new DFSP onboarded, we will have to amend the above-mentioned three reports to display the liquidity account information. And we expect there will be more information to display related to DFSPs, not only for the liquidity account number.

Example :

Fixing in JSON

We hardcoded information inside the report YAML file just like above.

Fix in Report

Display Bank ID or required information whenever it is needed.

2.3 Requested Behaviour

To be able to display DFSP's extra information such as liquidity accounts or others, we would like to request to add 2 new database tables in which we will keep the types of information (propertyType table) that each DFSP will have and, the values related to the type of the information based on DFSP (participantType table), and also add new APIs in /participant endpoint to create/update/delete these information from these 2 tables.

3. Proposed Solution Options

___

Upon onboarding each DFSP, Hub Operator can add the extra properties and their values using the participant's APIs. Below new APIs should be added into /participants endpoint and create new API endpoint which is /properties.

API Description
GET /properties

returns 200
To extract all the properties defined for all participant property
[{propertyId : "id_or_name_of_property", value : "some_value", "validationRegex : "REGEX"}]
GET /properties/{propertyId}

returns 200, 404
To get the property based on ID
POST /properties To create new generic property for all participants.
{propertyId : "id_or_name_of_property", value : "some_value", "validationRegex : "REGEX"}
PUT /properties/propertyId To update the generic property.
{value : "some_value", "validationRegex : "REGEX"}
DELETE /properties/{propertyId} To delete the generic property and it will also delete all the related property values from the participants
GET /participants/{name}/accounts/{accountId}/properties To extract all the properties and their values applied to the selected participant
GET /participants/{name}/accounts/{accountId}/properties/{propertyId} To get the value of the property of the selected participant.
POST /participants/{name}/accounts/{accountId}/properties/{propertyId} To map new property and its value to the selected participant. JSON request:{"value": "1234-5678-ABCD"}
DELETE /participants/{name}/accounts/{accountId}/properties/{propertyId} To delete the selected property from the account of the participant

Other APIs which need to return the properties information in their response.

  1. GET /participants
  2. GET /participants/{name}
  3. PUT /participants/{name}
  4. GET /participants/{name}/accounts
  5. POST /participants/{name}/accounts
@jgeewax
Copy link

jgeewax commented Aug 18, 2022

Some questions:

  1. How would an administrator use this API to retrieve properties for different DFSPs? Are these the /participants/name/accounts/accountID/properties/* URLs? Does this mean the /properties/* URLs are shortcuts for "the current participant and account ID?
  2. What would we lose by removing the regular expression validation ?
  3. When retrieving all properties, will we support pagination in some way in case this list of properties gets large?
  4. How many properties will be allowed in the quota? How big can each property be?
  5. Will property values always be strings? Or can they be more complicated (e.g., JSON objects)
  6. Why use POST to create a new and PUT to update? Why not always use PUT?
  7. In the current design, if a user tries to PUT to a property that doesn't exist, what happens?

@mdebarros
Copy link
Member

mdebarros commented Aug 19, 2022

Hey @jgeewax,

Some questions:

  1. How would an administrator use this API to retrieve properties for different DFSPs? Are these the /participants/name/accounts/accountID/properties/* URLs? Does this mean the /properties/* URLs are shortcuts for "the current participant and account ID?

I think there are two possibilities here:

  1. We both return the properties when we get the entity i.e. /participants/name/accounts/accountID (this returns the account + properties) and /participants/name/accounts/accountID/properties (this only returns the properties associated to the account)
  2. We maintain properties as a separate aspect of each entity that must be accessed via the /participants/name/accounts/accountID/properties/* end-point
  1. What would we lose by removing the regular expression validation?

As per the discussion this Wednesday with the Admin CCB members, it would seem that there is a lot of business value encoding some form of validation in the property-template itself.

However, as part of Wednesday's discussion, if we were to ignore the requirement for validation, then we should consider adopting the FSPIOP extensionList key-value pair approach instead since it's already being utilised by FSPIOP Hub and FSP operators.

  1. When retrieving all properties, will we support pagination in some way in case this list of properties gets large?

This is a good question. I am tempted to say we shouldn't as I don't expect there to be a "MASSIVE" list of properties...but that would not necessarily stop someone from using it as such.
I consider this property proposal very much a better version of the standard FSPIOP extensionList, which also could have the same "large" list problem.

  1. How many properties will be allowed in the quota? How big can each property be?
    Should we leave this up to the Hub scheme perhaps?
    @elnyry-sam-k is there a limit to the extensionList list size and key-value length?
  1. Will property values always be strings? Or can they be more complicated (e.g., JSON objects)

There is a good chance that we may need to store a JSON object in a value. The question here is should it be encoded as a "string" or left as an object?

For simplicity, looking at the extensionList, this is normally encoded as a string, but do we perhaps want to handle it differently here?

  1. Why use POST to create a new and PUT to update? Why not always use PUT?

I think it's more sticking to the semantics of the rest of the existing API. We generally use POST to create.

  1. In the current design, if a user tries to PUT to a property that doesn't exist, what happens?

I think there are two choices:

  1. allow for a generic property that has no defined property-template (i.e. regex, etc)
  2. throw some error to handle it

@mdebarros
Copy link
Member

Notes from CCB meeting held on 2022-08-17:

  • Admin CCB Members to review this Issue, and raise comments providing their suggested approach with regard to either:
    • Accept this proposal for the property approach or fall back to the existing FSPIOP extensionList approach.
  • Admin CCB Members discussed @jgeewax's discussion points at a high level as part of the discourse (some of these points have been added to the above questions raised by @jgeewax in this issue)

@mdebarros
Copy link
Member

mdebarros commented Aug 19, 2022

Here is my thought on this property approach:

  1. We should not support any "mutations" of existing properties, and instead use versioning (i.e. create a new property-template with the same name but a new version) of properties if any "changes" are required. This resolves the issue with existing properties as it would require Operators to instead migrate existing properties to a newer version if/when desired, instead of relying on some underlying process to migrate or handle changes to underlying property definitions.
  2. I am also of the opinion that if we do not require validation then I would prefer to stick to the existing FSPIOP extensionList approach due to the simplicity, and that it's widely being used in other Mojaloop APIs. If we do required validations then I am all for this proposal.

@mdebarros
Copy link
Member

mdebarros commented Sep 7, 2022

Update on 2022-09-07

  1. @jgeewax was not able to attend but send apologies.

  2. The following CCB Admin members agreed to move forward with a Solution Proposal:

    Note: Reminder that the details of the specification and overall design considerations would still need to be addressed as part of the Solution.

  3. @jgeewax to provide input if he is happy to proceed to the Solution Proposal stage

  4. Attending CCB members agreed that the Solution Proposal must include:

    • Guidance on how the Properties should and should not be used (i.e. abused). The main concern around the properties is that they would be utilised to store data that should not exist within the Core Mojaloop stack itself (e.g. customer data such as accounts) - i.e. external identifiers being stored internally. The general opinion is that these external identifiers should be managed/maintained outside of the Core Mojaloop stack itself, and rather handled by integration components/persistent-stores that are specific to a Scheme implementation.
    • @elnyry-sam-k to provide a note around the non-duplication of fields which would form part of the above guidance.

@elnyry-sam-k
Copy link
Member

elnyry-sam-k commented Sep 8, 2022

How many properties will be allowed in the quota? How big can each property be?
Should we leave this up to the Hub scheme perhaps?
@elnyry-sam-k is there a limit to the extensionList list size and key-value length?

@mdebarros - missed this earlier.. So yes, in the extensionList elements we do have limits. The key value pairs are limited to 16 (min:1, max:16).. with key's length limited to 32 characters and "value" limited to 128..

Swagger section for the extensionList:

@MichaelJBRichards
Copy link

Well, there's a difference here, isn't there? The extensionList is all included in a message, whereas properties (and their values) will be set via separate API calls. So there's no obvious reason to limit the number of properties. However, we might want to include versions of the endpoints which allow multiple properties, or multiple property values, to be set.

I assume, by the way, that we should include a currency in the API for setting property values if we plan to use it for account numbers, since they will be denominated in a single currency and there might be multiple currencies in a scheme.

I would recommend that, where we are setting property values via a POST, such as POST /participants/{name}/accounts/{accountId}/properties/{propertyId}) , we should not include information in the command string, but should move it to the body of the message. This is our usual practice, and it allows for more flexibility in the configuration of the parameter setting, where omitting a qualifier such as currency code means "for all currencies." We should be careful that we don't make all this definition too account-number-shaped.

@jgeewax
Copy link

jgeewax commented Sep 15, 2022

OK, just to make sure I'm not missing something big here, here's my understanding of this whole thing:

  1. Hub Operator will create a bunch of properties that participants can use for themselves (POST /properties) with regexes for the values that are allowed inside those values. (In truth, this isn't a "property" itself -- it's a "property specification" to be used when creating other properties w the same name scoped down to participants, right?
  2. Participants will update those property values for themselves as they go (PUT /participants/Me/accounts/1234/properties/MyProperty { value: "matches the regex"})

Is that... mostly correct?

If so, Dumb comment / question

Why do we have a "value" being stored when creating /updating a property (PUT /properties/MyProperty { value: ... , regex: ... })? Is that a default value? Or is that a mistake?

Can we possible update the table in this issue with the restrictions of who can access the various endpoints?

Limits and sizes

I think when defining limits in terms of characters for a global system, we have an i18n problem. I suggest we reference aip.dev/210 when defining limits of "32 characters". It basically says "character == code point; limit of x chars means max of 4*x bytes"

I do agree we shoudl have limits specifically because we return the entire map back in many places according to the spec listed here... If it was "1 property at a time", then the max keys limit would be mostly a non-issue... But either way, limits are in order...

PUT vs POST

I get that we generally think of POST for create and PUT for update and I definitely agree we should be consistent. However in this world, I don't even see a PUT for participant properties -- so what's the use case for updating a property value? Delete it and start again?

I'd argue that for all of this, we should get rid of POST (and the whole concept of "create") and stick instead with the idea that "the properties map always exists if the parent exists" and you are always updating keys in that map using PUT. (sort of like aip.dev/156).

Property value types

I'm +1 to always making the keys and values strings. Keeps things simple. People can do some encoding if they want more complicated values.

Versioning

When you say "versioning on properties" do you mean the property specifications (e.g., the hub operator's specs for properties)? Or the property values themselves for a given participant? This is starting to worry me more as it's quite complicated for something that is supposed to be exceedingly simple.


I'm really bummed I've missed this meeting 2x in a row now because I think I'm still very far away from being of the same mindset with the group on this topic... I'm hoping I'm just missing something obvious...

@mdebarros
Copy link
Member

mdebarros commented Sep 20, 2022

Meeting on 2022-09-14

Attendees

Notes

  • Consensus is to continue to the Solution Proposal stage and address any technical details or general issues there. (@tw-aungthawaye FYI).
  • @PaulGregoryBaker to follow up with @tw-aungthawaye.
  • Add wording/explanation that we should avoid duplication of identifiers
  • Add advice so that properties should not duplicate existing fields within the Core API services

@mdebarros
Copy link
Member

mdebarros commented Sep 20, 2022

OK, just to make sure I'm not missing something big here, here's my understanding of this whole thing:

  1. Hub Operator will create a bunch of properties that participants can use for themselves (POST /properties) with regexes for the values that are allowed inside those values. (In truth, this isn't a "property" itself -- it's a "property specification" to be used when creating other properties w the same name scoped down to participants, right?
  2. Participants will update those property values for themselves as they go (PUT /participants/Me/accounts/1234/properties/MyProperty { value: "matches the regex"})

Is that... mostly correct?

Yes, that is correct.

However, my suggestion is about adding versionning to each property and making the property-template-version immutable. This means that we always create a property-template-version (i.e. same property but with a new version). And each property that is instantiated always includes the version of the property "template". And the reason for this is to avoid dealing with the headache of what should we do if we allow an update to an existing property template that potentially invalidates the existing property instances. By using versioning we ensure that the existing property instances adhere to a specific version, and the "hub operator" would then need to "migrate" those property instances to the newer version (i.e. re-create the property instances).

For example....

Lets create following Property Template:

POST /properties

{
    "id": "ext-account-ref", <-- always a string
    "description": "short description of the v1.0 property",
    "version": 1.0, <-- assume semantic versioning
    "regex": "^[0-9]{8}$" <-- simple example, that the value should only be a string containing 8 numbers.
}

Then create an Property Instance on a specific account:

POST /participants/FSP1/accounts/123567/properties <-- something else that could be done here, is that if there is no existing Property Template (Id-Version combination), this operation could also create it behind the scenes for us. In which case we would need to provide the other details (i.e. regex, description, etc).

{
    "id": "ext-account-ref" <-- ID
    "version": 1.0, <-- this would be validated against the property-template that match the ID-Version combination
    "value": "12345678"
}

For querying, we have options, we could fetch all property versions (sorted with the latest version first), or the latest version...here is an example of returning all property versions...

GET /participants/FSP1/accounts/123567/properties/ext-account-ref

{
    [
        {
             "id": "ext-account-ref" <-- this is the "latest" version
             "description": "short description of the v1.0 property",
             "version": 1.0,
             "regex": "^[0-9]{8}$",
             "notes": "initial version",
             "value": "12345678"
        }
    ]
}

GET /participants/FSP1/accounts/123567/properties/ext-account-ref/1.0 <-- here is an example of fetching a specific property-version instance:

{
    "id": "ext-account-ref" <-- ID
    "description": "short description of the v1.0 property",
    "version": 1.0,
    "regex": "^[0-9]{8}$",
    "notes": "initial version",
    "value": "12345678"
}

Then perhaps there is a requirement to increase the ext-account-ref property value length from 8 to 16, then we would create a new version:

POST /properties

{
    "id": "ext-account-ref" <-- always a string
    "description": "short description of the v1.1 property",
    "version": 1.1, <-- always a number
    "notes": "increased size from 8 to 16",
    "regex": "^[0-9]{16}$" <-- simple example, that the value length should be 16 (previously 8).
}

Then create a Property Instance on a specific account :

POST /participants/FSP1/accounts/123567/properties

{
    "id": "ext-account-ref" <-- ID
    "version": 1.1,
    "value": "1234567812345678"
}

GET /participants/FSP1/accounts/123567/properties/ext-account-ref

{
    [
        {
             "id": "ext-account-ref" <-- latest entry always at position 0 (for convenience)
             "description": "short description of the v1.1 property",
             "version": 1.1,
             "regex": "^[0-9]{16}$",
             "notes": "increased size from 8 to 16",
             "value": "1234567812345678"
        },
        {
             "id": "ext-account-ref" <-- ID
             "description": "short description of the v1.0 property",
             "version": 1.0,
             "regex": "^[0-9]{8}$",
             "notes": "initial version",
             "value": "12345678"
        }
    ]
}

If required we could also delete the previous property-version --> DEL /participants/FSP1/accounts/123567/properties/ext-account-ref/1.0

PUT vs POST

I get that we generally think of POST for create and PUT for update and I definitely agree we should be consistent. However in this world, I don't even see a PUT for participant properties -- so what's the use case for updating a property value? Delete it and start again?

I'd argue that for all of this, we should get rid of POST (and the whole concept of "create") and stick instead with the idea that "the properties map always exists if the parent exists" and you are always updating keys in that map using PUT. (sort of like aip.dev/156).

I think this makes sense assuming we don't adopt the immutable versioning approach as per my suggestion above. But we should still need to address the issue around existing Property Instances that don't adhere to the updated/revised Property-Template?

For example:

Say we have the following Property Template:

PUT /properties/ext-account-ref

{
    "description": "short description of the property",
    "regex": "^[0-9]{8}$" <-- Set the initial regex to allow a string value containing only 8 numbers
}

PUT /participants/FSP1/accounts/123567/properties/ext-account-ref

{
    "value": "12345678"
}

Then we update the Property Template:

PUT /properties/ext-account-ref

{
    "description": "short description of the property",
    "regex": "^[0-9]{16}$" <--Update the regex from a string value containing only 8 to 16 numbers
}

Then what are we going to do about the existing property /participants/FSP1/accounts/123567/properties/ext-account-ref as it no longer adheres to the Property Template?

Versioning

When you say "versioning on properties" do you mean the property specifications (e.g., the hub operator's specs for properties)? Or the property values themselves for a given participant? This is starting to worry me more as it's quite complicated for something that is supposed to be exceedingly simple.

I'm really bummed I've missed this meeting 2x in a row now because I think I'm still very far away from being of the same mindset with the group on this topic... I'm hoping I'm just missing something obvious...

See my suggestion explanation/example above.

@jgeewax
Copy link

jgeewax commented Sep 26, 2022

I think you've pointed out sopme really important things that make me want to reinforce my points even further...

  1. If we support regexes, we should probably make these immutable. Making them mutable and validated at write-time seems like a bad idea -- it will lead to inconsistency in the data stored and confused people wondering why a value doesn't match the regex.
  2. I don't think we're talking about "versioning" but about "revisions" (similar, but not the same). Revisions on resources (in this case "Properties") is complicated and several months of work went into AIP-162 to work through all the edge cases that might come up. The proposal as you have it here seems very problematic to me. For example, depending on whether there are multiple versions, the API call GET /participants/FSP1/accounts/123567/properties/ext-account-ref would return an object sometimes (if there is only one version) and an array other times (if there is more than one version). I'm also unclear on the value of semantic versioning for something like properties (are people looking for "the latest of the v1 values"? that seems odd...) and think an opaque identifier is probably better. I'd also argue that revisions in this scenario are better suited for a world where we want change history (when did this value change? who did it? what was it before? how can I just revert it back to the old value? etc), but maybe I'm still misunderstanding.

I think it's also odd to me that we have "properties" to mean two different things. On the one hand we have "property specifications" that don't store an actual value but instead are a specification or template for values to be stored in the future. The other is a "property" that means what we all think it means. I'd argue we should use different names for these as they are fundamentally different things.

I'll put together an alternative proposal ASAP and send it as a reply here... Give me a few....

@jgeewax
Copy link

jgeewax commented Sep 27, 2022

Open API for FSP Interoperability - Change Request

1. Table of contents

2. Problem description

We'd like the ability for operators to define specific properties that participants could set to store additional per-participant data. For example, an operator may want to allow participants to store details of their respective liquidity accounts. The operator would like to define an arbitrary set of properties, with limits on the values that are allowed, and allow participants to fill in these values as needed.

Requirements:

  • Operators should be able to CRUD the property specifications.
  • Operators should be able to indicate the property validation rules.
  • Participants should be able to CRUD property values.
  • Property values should be included in some standard responses (e.g., GET /participants/{participantId})

3. Proposed solution

This all starts with creating two new resources:

Resource Description
PropertySpec A specification for a value that can be set for a given participant.
Property A value for a PropertySpec resource.

PropertySpec

A PropertySpec (short for "Property Specification") is a specification for a property to be eventually set with a value at the participant level. It can be thought of as a template for a property, with some extra metadata included (e.g., validation criteria).

Interface defintion

Field Type Description
propertyName string The ID of the property
validationRegex string A regex to be checked when values are set on the Property
createTime string (Output only) Timestamp of when this resource was created.
updateTime string (Output only) Timestamp of when this resource was last modified.

API methods

Method Path Description
GET /propertySpecs Lists all PropertySpec resources
GET /propertySpecs/{propertyName} Retrieves a PropertySpec by its name.
PUT /propertySpecs/{propertyName} Creates or updates a PropertySpec
DELETE /propertySpecs/{propertyName} Deletes a PropertySpec.

Property

A Property is a participant-level value that is set by each participant and conforms to the PropertySpec sharing the same propertyName.

Interface definition

Field Type Description
propertyName string The ID of the property
value string The value for this property, matching the validation defined in the PropertySpec.
createTime string (Output only) Timestamp of when this resource was created.
updateTime string (Output only) Timestamp of when this resource was last modified.

API methods

Method Path Description
GET /participants/{name}/accounts/{accountId}/properties Lists all Property resources.
GET /participants/{name}/accounts/{accountId}/properties/{propertyName} Retrieves a Property resource by its name.
PUT /participants/{name}/accounts/{accountId}/properties/{propertyName} Creates or updates a Property
DELETE /participants/{name}/accounts/{accountId}/properties/{propertyName} Deletes a Property

Behavior notes

Cascade

Since there is a referential relationship involved, it's important that all actions at the PropertySpec level cascade to the individual Property resources sharing the same propertyName. This means that if a PropertySpec is deleted, all Property resources that have the same propertyName should also be deleted.

Cascading updates for validation

Seeing that PropertySpec changes should cascade down to Property resources, it's possible that when updating the validation requirements for a PropertySpec the new validation wouldn't pass against existing Property values. In other words, what was a valid value before might not be after a change to the PropertySpec.

The obvious question is what we should do in this scenario. There are a few options:

  1. Don't allow changing of the validation for a PropertySpec at all.
  2. Only allow changing if the validation continues to pass for all existing
    Property values.
  3. Allow changing the validation even if values don't pass. In essence, only
    only apply validation at Property write-time.

The easiest to implement and least confusing for users is to make the validation fields of a PropertySpec immutable. If we're OK with making PropertySpec validation immutable, then this is by far the easiest option.

The next most straightforward option is to only apply validation at write-time. If we require the ability to update the validation parameters, then this is the best option.

The most complicated is to require existing values to pass the new and old validation. This is complicated as it requires a lock on the storage system, which introduces a large scaling bottleneck, limiting overall growth. As a result, this is discouraged.

Examples

Creating a new property specification

Request

PUT /properties/liquidityAccountNumber

{
    "validationRegex": "^[0-9]+$"
}

Response

{
    "propertyName": "liquidityAccountNumber",
    "validationRegex": "^[0-9]+$",
    "createTime": "2022-09-27T02:06:58.379Z",
    "updateTime": "2022-09-27T02:06:58.379Z"
}

(Note this response is the same for a GET /properties/liquidityAccountNumber request.)

Listing all property specifications

GET /properties

Response

[
    {
        "propertyName": "liquidityAccountNumber",
        "validationRegex": "^[0-9]+$",
        "createTime": "2022-09-27T02:06:58.379Z",
        "updateTime": "2022-09-27T02:06:58.379Z"
    }
]

Deleting a specific property specification

Request

DELETE /properties/liquidityAccountNumber

Response

200 OK

Setting a property value (create or update)

Request

PUT /participants/1234/accounts/5678/properties/liquidityAccountNumber

{
    "value": "12345678"
}

Response

{
    "propertyName": "liquidityAccountNumber",
    "value": "12345678",
    "createTime": "2022-09-27T02:11:50.052Z",
    "updateTime": "2022-09-27T02:11:50.052Z"
}

(Note that the result is the same when retrieving a property value by calling
GET /participants/1234/accounts/5678/properties/liquidityAccountNumber.)

Deleting a property value

Request

DELETE /participants/1234/accounts/5678/properties/liquidityAccountNumber

Response

200 OK

Edge case examples

Setting a property value with no matching specificiation

Request

GET /properties/missing

Response

404 Not Found

Request

PUT /participants/1234/accounts/5678/properties/missing

{
    "value": "12345678"
}

Response

400 Bad Request

Including property values in other API calls

The list of API calls that should include a "properties" field is:

  • GET /participants (on each individual participant)
  • GET /participants/{participantId}
  • (TBD more things here)

Request

GET /participants/1234

Response

{
    // Lots of other content
    "properties": [
        "propertyName": "liquidityAccountNumber",
        "value": "12345678",
        "createTime": "2022-09-27T02:11:50.052Z",
        "updateTime": "2022-09-27T02:11:50.052Z"
    ]
}

Outstanding questions

Updating validation

Should updating PropertySpec validation be permitted? Permitted only when the values would still pass the new validation? Or not permitted at all?

Proposal

Allow updating of validation, but only apply validation rules at write-time.

Revisioning

Is Property revisioning required? Or can we skip this for now?

Proposal

Skip this for now. When required, implement something along the lines of AIP-162.

References

@mdebarros
Copy link
Member

I think you've pointed out sopme really important things that make me want to reinforce my points even further...

  1. If we support regexes, we should probably make these immutable. Making them mutable and validated at write-time seems like a bad idea -- it will lead to inconsistency in the data stored and confused people wondering why a value doesn't match the regex.
  2. I don't think we're talking about "versioning" but about "revisions" (similar, but not the same). Revisions on resources (in this case "Properties") is complicated and several months of work went into AIP-162 to work through all the edge cases that might come up. The proposal as you have it here seems very problematic to me. For example, depending on whether there are multiple versions, the API call GET /participants/FSP1/accounts/123567/properties/ext-account-ref would return an object sometimes (if there is only one version) and an array other times (if there is more than one version). I'm also unclear on the value of semantic versioning for something like properties (are people looking for "the latest of the v1 values"? that seems odd...) and think an opaque identifier is probably better. I'd also argue that revisions in this scenario are better suited for a world where we want change history (when did this value change? who did it? what was it before? how can I just revert it back to the old value? etc), but maybe I'm still misunderstanding.

Some examples of where versioning would be useful:

  • Reports that reference a specific property <-- i.e. with an opaque identifier the report would need to be changed, whereas if it was versioned, we always fetch the latest version that exists for the account
  • It would be more aligned to a contract-based approach and would support a transaction/migration period for Ops/DFSPs, etc to manage these properties

I think it's also odd to me that we have "properties" to mean two different things. On the one hand we have "property specifications" that don't store an actual value but instead are a specification or template for values to be stored in the future. The other is a "property" that means what we all think it means. I'd argue we should use different names for these as they are fundamentally different things.

That is a great suggestion.

I'll put together an alternative proposal ASAP and send it as a reply here... Give me a few....

@mdebarros
Copy link
Member

mdebarros commented Oct 12, 2022

Open API for FSP Interoperability - Change Request

....

Outstanding questions

Updating validation

Should updating PropertySpec validation be permitted? Permitted only when the values would still pass the new validation? Or not permitted at all?

Proposal

Allow updating of validation, but only apply validation rules at write-time.

Revisioning

Is Property revisioning required? Or can we skip this for now?

Proposal

Skip this for now. When required, implement something along the lines of AIP-162.

References

@PaulGregoryBaker @tw-aungthawaye please would you take the time to review @jgeewax proposal (see #114 (comment)), and provide your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin-api-change-request A change request made to the Admin API
Projects
None yet
Development

No branches or pull requests

7 participants