Added content message delivery for SubscriptionWebsocketHandler

[ArtyomyuS]

Description

• This pull-request provides new support for the subscription R5 topic based to consider content property for the delivered messages so that if content is IDONLY the web socket message to contain an id, in case of FULLREFERENCE to consider the payload of the resource. In case of no content provided the payload remain as today: EMPTY. This pull-request does not affect previous FHIR versions nor any of the existing behaviour related to web socket messages.
• linked issues 5683

Quotes about findings and assumptions

• Not sure about the content type if that shall be covered as well inside this pull-request

Brief change log

• Updated ca.uhn.fhir.jpa.subscription.match.deliver.websocket.SubscriptionWebsocketHandler
• Changed the implementation for the method handleMessage(Message<?> theMessage)
• This pull-request doesn't not include unit tests for the new functionality. First draft review is required.

Verifying this change

This change added tests and can be verified as follows:

• To be done after first draft review

Documentation

• Does this pull-request introduce a new feature? Yes
• The documentation shall be part of the official one as: https://hl7.org/fhir/valueset-subscription-payload-content.html

[ArtyomyuS]

Hellow @jamesagnew , @fil512 , @tadgh could you please have an initial review, to confirm the pull-request is ok to be continued 👍 . Thanks

[jamesagnew]

@ArtyomyuS this approach looks good to me!

[ArtyomyuS]

Going to provide additional test 👍

[ArtyomyuS]

Hey @jamesagnew would you recommend update BaseSubscriptionDeliverySubscriberTest for the new changes or consider a new test class to fully support SubscriptionWebsocketHandler?

I challenge it as I can see the actual mocks within BaseSubscriptionDeliverySubscriberTest does not consider any of the SubscriptionWebsocketHandler implementations.

Please advice

[ArtyomyuS]

Hello @jamesagnew sorry if the question was not clear. Shall I create a new test class like: ca.uhn.fhir.jpa.subscription.match.deliver.websocket.SubscriptionWebsocketHandlerTest? Or shall I use an existing class for the payload content testing.

Thanks for your support.

[jamesagnew]

I would think putting a new test next to WebsocketWithSubscriptionIdR4Test would be the best spot - you could base your test largely on that test too

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2f5ffe7) 81.32% compared to head (151ec38) 83.39%.
Report is 452 commits behind head on master.

❗ Current head 151ec38 differs from pull request most recent head 39fac93. Consider uploading reports for the commit 39fac93 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5692      +/-   ##
============================================
+ Coverage     81.32%   83.39%   +2.06%     
- Complexity    23650    26888    +3238     
============================================
  Files          1425     1679     +254     
  Lines         86399   103818   +17419     
  Branches      11677    13155    +1478     
============================================
+ Hits          70265    86578   +16313     
- Misses        10947    11607     +660     
- Partials       5187     5633     +446     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ArtyomyuS]

Hello @jamesagnew , provided unit tests. I thought it shall be more appropriate to have them inside hapi-fhir-jpaserver-test-r5. Please advice if there are changes required. The pull-request is ready for review.

[jamesagnew]

Initial glance looks great! Once CI passes I'll give it a real review and get it merged. Thanks for the contribution!

[jamesagnew]

@ArtyomyuS looks like the spotless code formatting check failed. Can you run mvn spotless:apply and then commit+push the results to your branch?

[ArtyomyuS]

Hey @jamesagnew run the code, hopefully it works 🤞

[ArtyomyuS]

Hey @jamesagnew any clue why the mvn spotless:apply does not apply any changes on the current code?

Screen.Recording.2024-02-15.at.15.31.06.mov

[ArtyomyuS]

Also looking into the check pipeline it shows success: https://github.com/hapifhir/hapi-fhir/actions/runs/7906650465/job/21582427202?pr=5692#step:4:8399

[ArtyomyuS]

Hello @jamesagnew I guess we shall be fine now. Thank you

[ArtyomyuS]

Hey @jamesagnew any thought on the pull-request. Thank you for the support 🙏

[ArtyomyuS]

Hello @jamesagnew I can see we're progressing with the pull-request, is there anything I could help more here? 💪