search

belgif / rest-guide

REST Guidelines of Belgian government institutions
https://www.belgif.be/specification/rest/api-guide/
Apache License 2.0
24 stars 4 forks source link

Standardize validation issues on belgif types #126

Open pvdbosch opened 1 year ago

pvdbosch commented 1 year ago

Standardize common non-schemaViolation validation issues for belgif types. Currently CBSS has defined their own types:

Should we add the functional domain in their URN, corresponding to the related openapi schema (person-identifier, time, organization-identifier)? E.g.

jpraet commented 8 months ago

Updated issue type list of CBSS:

urn:problem-type:cbss:input-validation:replacedSsin
urn:problem-type:cbss:input-validation:canceledSsin
urn:problem-type:cbss:input-validation:invalidSsin
urn:problem-type:cbss:input-validation:unknownSsin
urn:problem-type:cbss:input-validation:invalidPeriod
urn:problem-type:cbss:input-validation:invalidIncompleteDate
urn:problem-type:cbss:input-validation:invalidYearMonth
urn:problem-type:cbss:input-validation:invalidEnterpriseNumber
urn:problem-type:cbss:input-validation:invalidEstablishmentUnitNumber

pvdbosch commented 8 months ago

Maybe a single issue type "invalid" suffices for all non-schema input structure validation issues? If the name of the input is returned, the client should be able to know which type it has and its associated rules. The detail property can still provide a specific explanation.

jpraet commented 8 months ago

That sounds like a good idea to us.

urn:problem-type:cbss:input-validation:invalidStructure would replace these:

... and the same issue can then also be used for other types.

We prefer to keep urn:problem-type:cbss:input-validation:invalidPeriod (end date before start date) separate.

Could we discuss moving these issue types from cbss to belgif namespace on the next workgroup?

pvdbosch commented 8 months ago

ok, added to agenda of next WG

jpraet commented 7 months ago

At CBSS we also introduced urn:problem-type:cbss:input-validation:invalidRefData to signal invalid reference data.

e.g. example /socialBenefits?socialBenefitCondition=BAD, where /refData/socialBenefitConditions/BAD does not exist.

pvdbosch commented 7 months ago

At CBSS we also introduced urn:problem-type:cbss:input-validation:invalidRefData to signal invalid reference data.

e.g. example /socialBenefits?socialBenefitCondition=BAD, where /refData/socialBenefitConditions/BAD does not exist.

This could be seen as a specific case of a resource identifier value that doesn't point to an existing resource. For path parameters, there's the 404 resourceNotFound problem, but we could also consider adding a general 'urn:problem-type:belgif:input-validation:resourceNotFound' issue type when identifier values in query params or request bodies don't match a resource.

jpraet commented 7 months ago

Having an issue type that can be used both for non-existing ref data and for other non-existing resource references sounds good to us. But reusing resourceNotFound for that can be a bit confusing.

How about urn:problem-type:belgif:input-validation:referencedResourceNotFound?

jpraet commented 6 months ago

Sometimes types are shared in multiple operations, but validation rules may be different depending on the situation. e.g. when you want to reuse a resource type for GET and POST, you may want to reject a POST where "id" property is filled in. Conversely, an input that is defined as optional in the OpenAPI definition, may be required in certain situations. e.g. a LocalizedString for which the "nl" and "fr" properties are required in certain situations.

To cover these, we have introduced these issue types at CBSS:

jpraet commented 3 months ago

As these urn:problem-type:cbss:input-validation: issue types are now also present in the Belgif rest-problem library, it would be good if they could be standardized under urn:problem-type:belgif:input-validation:

pvdbosch commented 3 months ago

WG discussion:

invalidStructure e.g. checksum invalidPeriod: bc two inputs (start+end) that may be split in query parameters

outOfRange : e.g. page number more than total of pages Should we return empty collection response "200" instead of a problem? => split to new issue. Scenarios:

referencedResourceNotFound: when performing a request to a resource with an identifier to another but it doesn't exist => unknownSsin is a specific occurrence of this one: can't it be generalized within referencedResourceNotFound?

replacedSsin, canceledSssin:

Should issue types related to business-specific schemas be documented in REST guide, or somewhere in fedvoc artifacts or next to the business-specific openapi schemas; but less easy to find them there? Naming convention of these issue types: include the ontology (= business domain) in the issue identifiers?

rejectedInput,requiredInput: permissive schemas that are reused in multiple operations, but should be more restricted according to the specific operation.

jpraet commented 3 months ago

referencedResourceNotFound: when performing a request to a resource with an identifier to another but it doesn't exist => unknownSsin is a specific occurrence of this one: can't it be generalized within referencedResourceNotFound?

CBSS RestDesignTeam agrees on using general referencedResourceNotFound issue type instead of unknownSsin.

jpraet commented 3 months ago

outOfRange : e.g. page number more than total of pages Should we return empty collection response "200" instead of a problem? => split to new issue. Scenarios:

  • concurrency - deleted items in between page consults
  • no total number of pages given in previous response, jump to later page immediately

Maybe an HTTP 400 Bad Request is indeed not the right approach when requesting a page beyond the number of available pages.

But doesn't urn:problem-type:belgif:input-validation:outOfRange seem generally useful regardless? e.g. for a date that cannot be in the past / future / ... and other runtime checks for dynamically determined ranges?