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/interlok 4149 jdbc retrystore #1254

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

jameswickham
Copy link
Contributor

Motivation

To split out the JDBC set of retry services so they are totally seperate from the exisiting services that use the filesystem and s3 retry stores. The reason for this is that while the services are both for "retrying" these JDBC set of services do a lot more that makes them incompatible with the existing services(acknowledgments, expired messages, etc).

Modification

Renamed the classes and moved them to the appropriate packages.

PR Checklist

  • been self-reviewed.
  • Added javadocs for most classes and all non-trivial methods
  • Added supporting annotations (like @XStreamAlias / @ComponentProfile)
  • Added DefaultValue annotation when there is a default value (e.g. @DefaultValue('true'))
  • Added validation annotation (@NotNull...) when required and add @Valid when nested class has some validation
  • Checked that @NotNull and @notblank annotations have a meaningful message when appropriate.
  • [n/a] Checked that new 3rd party dependencies are appropriately licensed
  • Added comments explaining the "why" and the intent of the code wherever it would not be obvious for an unfamiliar reader
  • Added unit tests or modified existing tests to cover new code paths
  • Tested new/updated components in the UI and at runtime in an Interlok instance
  • Reviewed java class members so that missing annotations are added (InputFieldDefault/ComponentProfile etc)
  • Checked that javadoc generation doesn't report errors
  • Checked the display of the component in the UI
  • [n/a] Remove any config/license annotations
  • [n/a] Check the gradle build file to make sure the dependencies section is more explicit "implementation/api".

Result

The services that were named as follows:
StoreMessageForRetryService
RetryMessageService
AcknowledgmentService

Are all now prefixed with 'Jdbc' and you no longer have the option to select a retry store as it is expected to ONLY be a JDBC retry store. Defining the SQL properties file is now in the main service form and not within the retry store.

All functionality remains exactly the same.

However this is a config breaking change due to the restructure and renaming.

Testing

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3c9b197) 93.47% compared to head (292344e) 93.27%.
Report is 4 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #1254      +/-   ##
=============================================
- Coverage      93.47%   93.27%   -0.20%     
- Complexity     10852    10915      +63     
=============================================
  Files           1054     1058       +4     
  Lines          29104    29434     +330     
  Branches        2038     2069      +31     
=============================================
+ Hits           27204    27454     +250     
- Misses          1484     1534      +50     
- Partials         416      446      +30     
Files Coverage Δ
...s/core/http/client/net/HttpRequestServiceImpl.java 100.00% <ø> (ø)
...is/core/http/jetty/retry/FilesystemRetryStore.java 100.00% <ø> (+6.45%) ⬆️
...com/adaptris/core/http/jetty/retry/RetryStore.java 100.00% <ø> (ø)
...re/services/jdbc/retry/JdbcAcknowledgeService.java 64.28% <100.00%> (ø)
.../core/services/jdbc/retry/JdbcRetryServiceImp.java 77.71% <100.00%> (ø)
...es/jdbc/retry/JdbcStoreMessageForRetryService.java 87.09% <100.00%> (ø)
.../services/jdbc/retry/JdbcRetryMessagesService.java 88.15% <93.33%> (ø)

... and 12 files with indirect coverage changes

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

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

Successfully merging this pull request may close these issues.

1 participant