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

@OperationParam handling feels incorrect #317

Closed
ohr opened this Issue Mar 21, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@ohr
Collaborator

ohr commented Mar 21, 2016

I'm wondering why DataTypes needs to be provided as parameters for extended operations rather than subclasses of BaseParam just like in @Search methods. The code in OperationParameter#initializeTypes also looks quite inconsistent with regard to this (e.g. why DateRangeParam is allowed, but no other Params...)

Related to this, there is also a potential misconception about what complex vs. primitive parameters are with respect to allowing them for GET operations. According to https://chat.fhir.org/#narrow/stream/implementers/topic/operations.20with.20%27complex%27.20parameters, e.g. a TokenParam is not considered complex although carries more than a single primitive value.

For implementing operations with TokenParams, I need to work around this by providing a StringType parameter and split the string at the pipe into system and value part. Feels very hacky...

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Mar 26, 2016

This is very timely really.

The main reason for the use of datatypes instead of search parameter types is that the "parameters-inthe-URL" syntax for calling operations is just a shortcut for the more formal way, which is sending in the values in a Parmeters resource. Since you need to be able to send in values as fields in a resource instance, the values need to be datatypes.

Grahame has always expressed a preference that we should be able to use search parameter types too, but it was never clear how this would translate to the parameters resource. The FHIR Infrastructure group finally voted on this about a month ago maybe, and the values will be represented as String types in the Parameters.

So this means HAPI needs to catch up. I'd agree you should be able to use all the same types on operations, e.g. StringParam, TokenParam. I spent some time on a train ride this morning getting that working, so I'll check that in shortly.

@ohr

This comment has been minimized.

Collaborator

ohr commented Mar 31, 2016

Not sure if this adds to this issue...
From the changeset, it looks like the IQueryParameter parameters do not allow the operation to be called as GET request.
There was a discussion on the chat (https://chat.fhir.org/#narrow/stream/implementers/topic/operations.20with.20%27complex%27.20parameters) indicating that standard parameters (such as a TokenParam) are considered "primitive" and are allowed for GET requests on operations.

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Mar 31, 2016

They definitely should work...

Here is an example of a unit test method which tests an HTTP GET with a StringParam and TokenParam operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment