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

Feature Request: Add client support for $process-message special oper… #542

Merged
merged 6 commits into from Mar 17, 2017

Conversation

@hugosoares
Copy link
Contributor

@hugosoares hugosoares commented Jan 18, 2017

…ation #407

Changed client to send messages to a processing server:
Object response = client
.operation()
.onServer()
.processMessage() // New operation for sending messages
.setResponseUrlParam("http://myserver/fhir") //optional url parameters to send with the POST
.setAsyncProcessingMode()//optional url parameters to send with the POST
.setMessageBundle(msgBundle) // the Message bundle to send
.execute();

hugosoares added 3 commits Jan 18, 2017
…ation #407

Changed client to send messages to a processing server:
Object response = client
.operation()
.onServer()
.processMessage()
.setResponseUrlParam("http://myserver/fhir")
.setMessageBundle(msgBundle)
.execute();
@hugosoares
Copy link
Contributor Author

@hugosoares hugosoares commented Jan 20, 2017

Closing an reopen to trigger travis

@hugosoares hugosoares closed this Jan 20, 2017
@hugosoares hugosoares reopened this Jan 20, 2017
Copy link
Collaborator

@jamesagnew jamesagnew left a comment

Hi Hugo,

Sorry to take so long to review this, I needed to educate myself a bit on how $process-message actually works before I felt qualified to review :)

I think this looks awesome, and I'm looking forward to merging. I have a few comments on the design, definitely nothing major, but would love to your your thoughts on them.

@@ -76,6 +76,8 @@
private Boolean myPrettyPrint = false;
private SummaryEnum mySummary;
private final String myUrlBase;
private Boolean myAsync = null;

This comment has been minimized.

@jamesagnew

jamesagnew Jan 31, 2017
Collaborator

I'm wondering if it might make more sense to not make these things global properties of the client, and instead make them in OperationMethodBinding#createProcessMsgInvocation?

I think that would be a bit more consistent with our general approach of making settings which are specific to a particular type of operation be non-global.

This comment has been minimized.

@hugosoares

hugosoares Jan 31, 2017
Author Contributor

Makes perfect sense. I Will make that change.

@@ -24,5 +24,6 @@
public interface IOperationUnnamed {

IOperationUntyped named(String theName);
IOperationProcessMsg processMessage();

This comment has been minimized.

@jamesagnew

jamesagnew Jan 31, 2017
Collaborator

I'm thinking a javadoc link to the messaging part of the FHIR spec would be a useful addition here if possible

This comment has been minimized.

@jamesagnew

jamesagnew Jan 31, 2017
Collaborator

Actually, I'm also wondering if it might make sense to move this method from IOperationUnnamed over to IOperation.

Technically with it on IOperationUnnamed you could invoke this operation at the type or instance level, but FHIR doesn't allow that so the API should probably try to prevent it too.

This comment has been minimized.

@hugosoares

hugosoares Jan 31, 2017
Author Contributor

Makes perfect sense. I Will make that change.

*
* @author HGS
*/
public interface IOperationProcessMsg<T extends IBaseResource> extends IClientExecutable<IOperationProcessMsg<T>, T> {

This comment has been minimized.

@jamesagnew

jamesagnew Jan 31, 2017
Collaborator

You could probably add typing here, in order for client code to avoid the need to cast the result of the execute() method later.

E.g. add one more interface to the chain so that client calls look like:

Bundle response = client
       .operation()
       .onServer()
       .processMessage()
       .setResponseUrlParam("http://myserver/fhir")
       .setMessageBundle(msgBundle)
       .synchronous(Bundle.class)
       .execute();

...and....

OperationOutcome response = client
       .operation()
       .onServer()
       .processMessage()
       .setResponseUrlParam("http://myserver/fhir")
       .setMessageBundle(msgBundle)
       .asynchronous(OperationOutcome.class)
       .execute();

This comment has been minimized.

@hugosoares

hugosoares Jan 31, 2017
Author Contributor

Nice tip. I Will make that change.

hugosoares added 3 commits Feb 9, 2017
# Conflicts:

#	hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/GenericClient.java
@hugosoares
Copy link
Contributor Author

@hugosoares hugosoares commented Feb 9, 2017

Hi James,
I just commited the changes requested.
Can you merge this in time for the next release?
We have some clients in need of this feature that are going live soon. It would be great the could upgrade their hapi lib and use this instead of an ugly work around they use right now.

@hugosoares hugosoares closed this Feb 9, 2017
@jamesagnew jamesagnew reopened this Feb 10, 2017
@jamesagnew jamesagnew merged commit da9daf8 into hapifhir:master Mar 17, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@jamesagnew
Copy link
Collaborator

@jamesagnew jamesagnew commented Mar 17, 2017

Hi Hugo,

This has now been merged. Thanks again for the contribution!

jamesagnew added a commit that referenced this pull request Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants