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

CS-11: Add a plugin to cache repeated results #16

Merged
merged 20 commits into from
Nov 8, 2022

Conversation

marian13
Copy link
Owner

@marian13 marian13 commented Nov 7, 2022

What was done as a part of this PR?

  • I̶n̶t̶r̶o̶d̶u̶c̶e̶d̶ ̶t̶h̶e̶ ̶̶C̶a̶n̶A̶d̶j̶u̶s̶t̶F̶o̶r̶e̶i̶g̶n̶R̶e̶s̶u̶l̶t̶̶ ̶p̶l̶u̶g̶i̶n̶ ̶t̶h̶a̶t̶ ̶p̶r̶o̶v̶i̶d̶e̶s̶ ̶̶r̶e̶s̶u̶l̶t̶_̶o̶f̶̶.̶
  • Introduced the CachesRepeatedResults plugin that caches results that are executed multiple times for one entry service.

Why it was done?

How to test?

Notes

  • This PR is delayed since it is difficult to make a final decision on how exactly to implement this feature.

  • Convenient Service has no global context (this is probably one of the main reasons why Convenient Service was born).
    As a result, there is no way to share the results cache between different services like context in React.

  • Similar approach as in the Active Record Query Cache is probably the way to go, but it requires management of multithreading issues. Also it is not currently obvious when to create and destroy(refresh) the cache. Should it be delegated to the end user?

  • Rails Current Attributes integration can be implemented since it already has some solutions for the problems listed above.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3411166670

  • 23 of 23 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.006%) to 95.86%

Totals Coverage Status
Change from base Build 3410798751: -0.006%
Covered Lines: 5325
Relevant Lines: 5555

💛 - Coveralls

@coveralls
Copy link

coveralls commented Nov 7, 2022

Pull Request Test Coverage Report for Build 3414920048

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 84 of 92 (91.3%) changed or added relevant lines in 11 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 95.762%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/convenient_service/rspec/helpers/custom/wrap_method/errors.rb 4 6 66.67%
lib/convenient_service/service/plugins/caches_repeated_results/middleware.rb 8 14 57.14%
Totals Coverage Status
Change from base Build 3413878735: -0.1%
Covered Lines: 5355
Relevant Lines: 5592

💛 - Coveralls

@marian13 marian13 self-assigned this Nov 7, 2022
@marian13 marian13 force-pushed the CS-11_plugin_to_cache_repeated_results branch from 672f701 to ef7f58c Compare November 7, 2022 21:33
@marian13 marian13 force-pushed the CS-11_plugin_to_cache_repeated_results branch from ef7f58c to ae01aa8 Compare November 7, 2022 21:34
@marian13 marian13 force-pushed the CS-11_plugin_to_cache_repeated_results branch from 012a2da to 7674afe Compare November 8, 2022 14:13
@marian13 marian13 merged commit 9b7aa0e into main Nov 8, 2022
@marian13
Copy link
Owner Author

marian13 commented Nov 8, 2022

This PR was not supposed to be merged.

GitHub decided to change its status since #17 was merged.

CS-12_cache_return_value_enhancements branch was created from CS-11_plugin_to_cache_repeated_results.

It contained the same amount of commits +1.

That is the reason.

@marian13 marian13 deleted the CS-11_plugin_to_cache_repeated_results branch November 8, 2022 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants