-
Notifications
You must be signed in to change notification settings - Fork 40
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
Comments
Some questions:
|
Hey @jgeewax,
I think there are two possibilities here:
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
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.
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?
I think it's more sticking to the semantics of the rest of the existing API. We generally use POST to create.
I think there are two choices:
|
Notes from CCB meeting held on 2022-08-17:
|
Here is my thought on this
|
Update on 2022-09-07
|
@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: mojaloop-specification/fspiop-api/documents/v1.1-document-set/fspiop-v1.1-openapi3.yaml Line 2794 in 87b78af
|
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. |
OK, just to make sure I'm not missing something big here, here's my understanding of this whole thing:
Is that... mostly correct? If so, Dumb comment / questionWhy do we have a "value" being stored when creating /updating a property ( Can we possible update the table in this issue with the restrictions of who can access the various endpoints? Limits and sizesI 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 POSTI 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 typesI'm +1 to always making the keys and values strings. Keeps things simple. People can do some encoding if they want more complicated values. VersioningWhen 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... |
Meeting on 2022-09-14AttendeesNotes
|
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:
{
"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:
{
"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...
{
[
{
"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"
}
]
}
{
"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
{
"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 :
{
"id": "ext-account-ref" <-- ID
"version": 1.1,
"value": "1234567812345678"
}
{
[
{
"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 -->
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:
{
"description": "short description of the property",
"regex": "^[0-9]{8}$" <-- Set the initial regex to allow a string value containing only 8 numbers
}
{
"value": "12345678"
} Then we update the Property Template:
{
"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
See my suggestion explanation/example above. |
I think you've pointed out sopme really important things that make me want to reinforce my points even further...
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.... |
Open API for FSP Interoperability - Change Request1. Table of contents
2. Problem descriptionWe'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:
3. Proposed solutionThis all starts with creating two new resources:
|
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:
- Don't allow changing of the validation for a
PropertySpec
at all. - Only allow changing if the validation continues to pass for all existing
Property
values. - Allow changing the validation even if values don't pass. In essence, only
only apply validation atProperty
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
- AIP-162: Resource revisions
Some examples of where versioning would be useful:
That is a great suggestion.
|
@PaulGregoryBaker @tw-aungthawaye please would you take the time to review @jgeewax proposal (see #114 (comment)), and provide your comments. |
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
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 :
Settlement Initiation 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 :
We hardcoded information inside the report YAML file just like above.
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
.GET /properties
returns 200
[{propertyId : "id_or_name_of_property", value : "some_value", "validationRegex : "REGEX"}]
GET /properties/{propertyId}
returns 200, 404
POST /properties
{propertyId : "id_or_name_of_property", value : "some_value", "validationRegex : "REGEX"}
PUT /properties/propertyId
{value : "some_value", "validationRegex : "REGEX"}
DELETE /properties/{propertyId}
GET /participants/{name}/accounts/{accountId}/properties
GET /participants/{name}/accounts/{accountId}/properties/{propertyId}
POST /participants/{name}/accounts/{accountId}/properties/{propertyId}
{"value": "1234-5678-ABCD"}
DELETE /participants/{name}/accounts/{accountId}/properties/{propertyId}
Other APIs which need to return the properties information in their response.
The text was updated successfully, but these errors were encountered: