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

Annotating the GET /entity (ie GetCollection) query for a resource that extends %pkg.isc.rest.model.resource #14

Open
ghost opened this issue Jul 21, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@ghost
Copy link

ghost commented Jul 21, 2022

Hi, we have defined a resource that extends %pkg.isc.rest.model.resource. We have our own custom query parameters that our resource uses in

GetCollection(ByRef params,  selectMode As %Integer)

We need to document this method so that we can describe the set of valid query parameters.
How can we document this method so that the Open API generator will produce this custom documentation?

Can we create an action map whose name is null?

<actions>
<action name="" target="CurrentClass" call="GetCollection">
	<argument name="myURLparam" target="param" source="query"/>
</action>
</actions>
@ghost
Copy link
Author

ghost commented Jul 21, 2022

Looks like this has been considered:

} catch (ex) {
// TODO: Come up with some way to handle classes that don't use DBMappedResource methods, instead of (incorrectly) listing them as having no parameters.
// TODO: The following methods theoretically perform known operations:
// GetCollection, GetModelInstance, DeleteModelInstance, HandleInvokeClassAction, HandleInvokeInstanceAction
// In practice, classes can overwrite their behavior (possibly in non-standard ways), such as %pkg.isc.rest.model.environment, which returns a single static object (not wrapped in an array) from the Query endpoint.
// Ultimately there's no way to properly handle the range of these modifications without actually running the methods in question, which I'm trying to avoid for the following reasons:
// 1) Wrt. the HandleInvoke methods, they may cause side-effects that cannot be rolled-back with transactions
// 2) There is no way to ensure spec accuracy by testing the methods directly, and code-analysis is infeasable in the general case (ignoring its NP-completeness, it would take *forever* to implement with any degree of accuracy)
// Fortunately, it should be relatively easy to detect when classes have overwritten the methods.
// At the moment, my thought is that the best way to handle this is to do some basic analysis of the "safe" to call methods to make minor modifications to the expected spec, and mark the endpoints as possibly-incorrect somehow
// (I will need to look into the OpenAPI spec to see how best to do this). For methods whose responses cannot be reconciled with the expected responses at all, it may suffice to generate schemas based upon their responses and mark
// them as incorrect/unsafe as well. For methods that cannot be safely called at all, I need to see if there is any way to mark information as Unknown in the OpenAPI spec (so far as I know, there isn't, so I might need to get
// somewhat creative there).
// --> What to do about parameters in the case where the method has been overwritten??
// UPDATE: An initial version of this is implemented with GetCollection()
Set parametersIterator = {}.%GetIterator()
}

We might go ahead and add the documentation in here

/// Modify the autogenerated OpenAPI Specification objects for this class by changing the values of the passed ByRef parameters
/// @Argument schemas: List of %pkg.isc.rest.openAPI.model.schema
/// @Argument endpoints: Array of endpoint URL -> %pkg.isc.rest.openAPI.model.pathItem (%pkg.isc.rest.openAPI.model.paths)
/// Sample use cases:
///   - Modify resource schema descriptions
///   - Add additional ISC_* fields to the resource schemas
///   - Correct resource schemas for classes that use custom JSON generation code
///   - Remove non-desired endpoints from output
///   - Provide alternate schemas for endpoint input / output (ie. in the case of %DynamicObject use)
///   - Add documentation for non-standard endpoints
/// Note: Endpoints can be removed from the endpoints array as desired, however schemas should not be removed from the schemas array.
ClassMethod ModifyOpenAPIInfo(ByRef schemas As %ListOfObjects, ByRef endpoints As %pkg.isc.rest.openAPI.model.paths) [ Abstract ]
{
}

This might be harder than it sounds by why not have an optional XDATA block that allows you to document query parameters as you would in a custom action?

@isc-tleavitt isc-tleavitt added the enhancement New feature or request label Jul 22, 2022
@isc-tleavitt
Copy link
Collaborator

I think the ideal solution here would be to have "Schemas" and "Endpoints" XData blocks (containing JSON, not XML, obviously ;)) that end up getting merged in to the OpenAPI document. The default implementation of ModifyOpenAPIInfo could be updated to check for these.

@isc-pbarton @isc-kiyer would you agree?

@isc-kiyer
Copy link
Collaborator

@isc-tleavitt That sounds reasonable to me
cc @isc-pbarton

@isc-pbarton
Copy link

Yes agreed on my end. Specifying JSON that gets merged into the document will be simpler and more transparent than overriding the ModifyOpenAPIInfo method.

@isc-tleavitt
Copy link
Collaborator

Brought inside (internal reference: APPS-13700)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants