Resolve backend operations via the API documentation - #64
Conversation
There was a problem hiding this comment.
Hi @mihailefter this looks really good. 🙂
I do have a few questions/suggestions.
Details are in the comments, but, in summary:
- Note that cached api-docs need to be refreshed if a
ResourceDefinitionis created or updated. This may be good to remember when implementing the corresponding admin functionality later. - Looks like
fdpApi.tshas some repetition that could be replaced by a genericperformOperation()function? See comment for detailed example. - The changes are more complex than I expected, due to the
asynchandling of operations withoperationBindingetc. Would it be possible to simplify by awaiting the api-docs at the very start, (likeloadClientConfig) and then handling operations synchronously? I would think that there's not much to do anyway if api-docs fail to load. I do like the async implementation, but my main concern is complexity.
| const subjectUri = resolveSubjectUri(store, rootUri) | ||
| const declaredUrls = subjectUri ? getNodeRefs(store, subjectUri, DCAT_ENDPOINT_DESCRIPTION) : [] | ||
| const fallbackUrl = new URL('/v3/api-docs', rootUri).toString() | ||
| return [...new Set([...declaredUrls, fallbackUrl])] |
There was a problem hiding this comment.
Although JavaScript Set preserves insertion order, in many other languages this is not guaranteed.
Maybe a comment to point this out?
| return [...new Set([...declaredUrls, fallbackUrl])] | |
| // Note that JavaScript Set preserves insertion order | |
| return [...new Set([...declaredUrls, fallbackUrl])] |
| type OpenApiOperation = { operationId?: string } | ||
| type OpenApiDoc = { paths?: Record<string, Record<string, OpenApiOperation>> } | ||
|
|
||
| function isOpenApiDoc(doc: unknown): doc is OpenApiDoc { |
There was a problem hiding this comment.
| function isOpenApiDoc(doc: unknown): doc is OpenApiDoc { | |
| /** Duck-typing: If it looks like an OpenApiDoc, treat it as one. */ | |
| function isOpenApiDoc(doc: unknown): doc is OpenApiDoc { |
|
|
||
| let apiDocsPromise: Promise<unknown> | null = null | ||
|
|
||
| /** Fetches the FDP's OpenAPI doc once per session and reuses it for all subsequent lookups. */ |
There was a problem hiding this comment.
Note that the API docs are (supposed to be) updated by the backend whenever a ResourceDefinition is added or changed.
That means the client should refresh the API docs after creating/editing ResourceDefinition objects.
| const userId = computed(() => (route.params.id as string | undefined) ?? 'current') | ||
|
|
||
| /** | ||
| * Self-service profile routes use current-user operations. Admin routes use uuid-based user |
There was a problem hiding this comment.
It is not immediately clear to me what "self-service" and self refer to.
Does it refer to current user?
There was a problem hiding this comment.
Looks like there's quite a bit of repetition in this file.
Perhaps you could define a generic method performOperation(<operation-id>, <data>) that performs the actual request, based on operation details, and call that method from the relevant locations.
Using searchResources() as an example, the following code from SearchView.vue
const { url, method } = await searchBinding
results.value = (await searchResources(q, url, method)) as SearchResult[]could then be replaced by something like (please excuse the sloppy pseudo-code):
operationResult = (await performOperation(
<search-operation-id>,
<object-containing-query-string-and-other-relevant-data>
)) as OperationResult
results.value = ... // extract SearchResult[] from operationResultA similar approach applies to all the other functions.
To illustrate the idea, here's an example from one of my Python-based FDP clients (synchronous instead of async):
class APIClient(object):
...
def release_schema_version(
self, uuid: str, version: str, description: str = "", public: bool = False
) -> OperationResult:
"""Creates a metadata-schema-version by releasing the metadata-schema-draft"""
# minimal post body
metadata_schema_version = {
"description": description,
"published": public,
"version": version,
}
# perform operation
return self.perform_operation(
key="releaseSchemaVersion", uuid=uuid, json=metadata_schema_version
)
...
def perform_operation(self, key: str, **kwargs) -> OperationResult:
"""
Performs an operation defined in the OpenAPI docs.
Path parameters must be speficied as kwargs, e.g. uuid=<string>. Additional
kwargs, if any, are passed on to the requests method call, e.g. json=<dict>.
"""
# get operation info
try:
operation = self.api_operations[key]
logger.info("performing operation: %s", key)
except KeyError as e:
logger.error("unknown operation: %s", key)
self.list_operations()
raise e
# remove path parameters from kwargs and format uri
path_parameters = {
parameter_name: kwargs.pop(parameter_name, None)
for parameter_name in self._get_api_parameters(
operation=operation, param_type="path"
)
}
url = self.url + operation["uri"].format(**path_parameters)
# perform request
response = getattr(self.session, operation["method"])(url=url, **kwargs)
# handle response
if response.ok:
# handle content type
content_type = response.headers.get("content-type")
if "json" in content_type:
content = response.json()
elif content_type.startswith("text"):
# same as response.content.decode("utf-8")
content = response.text
else:
logger.warning("unknown content-type: %s", content_type)
content = response.content
logger.info("operation successful: %s", key)
logger.debug("result: %s", content)
return OperationResult(
location=response.headers.get("location"),
content_type=content_type,
content=content,
)
self._log_api_operation_requirements(operation=operation)
raise Exception(
f"operation failed: {key}\n\t"
f"request: {response.request.method} {response.request.url} "
f"{response.request.body}\n\t"
f"response: {response.content or '-'}"
)There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
|
From the PR description above:
@mihailefter i think the current PR is a great start. This advanced stuff can be done at a later stage. Perhaps good to keep this in mind when implementing the admin functionality for "resource definitions" and "metadata schemas?" |
2 participants
In line with #34, the client now stops hardcoding API paths. Instead, it tries to discover the FDP's API document via
dcat:endpointDescription, provided in the root Turtle response by FDP 1.22+ (see FAIRDataTeam/FAIRDataPoint#952). If no usable document is declared there, it falls back to the/v3/api-docsguess. If the document is found, requests are resolved byoperationId(bindOperation) instead of hardcoded paths.As a result, affected UI elements (buttons, menu links, forms) are only shown when the connected FDP's API document offers the corresponding operation. I also added route guards for
/login,/users,/users/create,/users/:id,/users/current, and/search, so direct navigation cannot bypass the hidden UI and hit a raw operation-resolution failure.If no usable API document is found at all, the affected UI simply stays hidden, with no explicit message shown for that case yet.
Since the discovery flow depends on the root URI from runtime config, I also added a visible startup error instead of a blank page for when that config fails to load.
The following things are still open:
Capability-gated route guards currently redirect silently to
/with no explanation. Worth discussing whether direct navigation to an unavailable route should tell the user why (e.g. "this FDP does not support user management") rather than just bouncing home. The same applies more broadly: if the API document itself can't be found, there's no visible indication of that either, just an app that quietly offers less.The API document also carries
requestBody/parametersschemas (required fields, formats, enums) that could eventually drive client-side form validation instead of the current hardcoded checks.