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

Fix cached data in ProcessBillingBatchService #166

Merged
merged 2 commits into from
Mar 14, 2023

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Mar 14, 2023

https://eaflood.atlassian.net/browse/WATER-3906

As part of implementing Handle errors in ProcessBillingBatchService we refactored the ProcessBillingBatchService to make the code cleaner and easier to read.

One of the changes was moving the generatedInvoices and generatedInvoiceLicences to the module scope rather than just in the method scope. It saved us from having to pass them around the place which removed a bunch of 'cruft'. What we didn't take into account is they would be cached between calls to the service.

Now it's at module scope the data from previous runs is being cached and still exists when a new one starts. So, we're getting to the finalise stage and we're trying to persist old data from a previous run!

This change resolves the issue.


For reference, we move the variables into the scope of the go() function removing the issue of the data being cached at the global scope.

We then take a halfway house approach; we pass the information into our methods but we accept having to mutate the data rather than pass back a modified copy.

Screenshot of logs showing caching

Screenshot 2023-03-14 at 10 30 20

@Cruikshanks Cruikshanks added the bug Something isn't working label Mar 14, 2023
@Cruikshanks Cruikshanks self-assigned this Mar 14, 2023
@Cruikshanks Cruikshanks marked this pull request as ready for review March 14, 2023 11:45
@Cruikshanks
Copy link
Member Author

The SonarCloud fail is now just code coverage, and that is due to our decision to hold off writing unit tests until the ProcessBillingService stabilises. This has been proven by the fact we've made large-scale changes to it in this PR, which is why SonarCloud is back with a vengeance!

So, we're ignoring it for now but do intend to write full unit tests in the near future.

https://eaflood.atlassian.net/browse/WATER-3906

As part of implementing [Handle errors in ProcessBillingBatchService](#161) we refactored the `ProcessBillingBatchService` to make the code cleaner and easier to read.

One of the changes was moving the `generatedInvoices` and `generatedInvoiceLicences` to the module scope rather than just in the method scope. It saved us having to pass them around the place which removed a bunch of 'cruft'. What we didn't take account of is they would be cached between calls to the service.

Now it's at module scope the data from previous runs is being cached and still exists when a new one starts. So, we're getting to the finalise stage and we're trying to persist old data from a previous run!

This change resolves the issue.
We move the information into the scope of the `go()` function removing the issue of the data being cached at the global scope.

We then take a half-way house approach; we pass the information into our methods but we accept having to mutate the the data rather than pass back a modified copy.
@Cruikshanks Cruikshanks force-pushed the fix-cached-info-in-process-billing branch from 0ff81f4 to 5d2148b Compare March 14, 2023 11:50
@Cruikshanks Cruikshanks merged commit d97d290 into main Mar 14, 2023
@Cruikshanks Cruikshanks deleted the fix-cached-info-in-process-billing branch March 14, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants