THREESCALE-11631: Delete usage limits for plans when deleting backend API usage #3996
Conversation
mayorova
force-pushed
the
backend-level-plan-limits
branch
from
f3047ee to
eb63b8d
Compare
mayorova
force-pushed
the
backend-level-plan-limits
branch
from
dcbe2e0 to
a8374f7
Compare
There was a problem hiding this comment.
I followed the verification steps and everything worked as expected for me. Just a few questions:
- Apparently when I add the usage limit, that's pushed directly to backend, is that correct?
- What if I remove the backend and then the limits are removed, are they removed also from backend?
- Does the "Integration -> Promote" button interfere with usage limits somehow? I mean, Can I cause inconsistencies by adding and removing backends and usage limits without pushing the button?
| scope :of_plan, ->(plan) { where(:plan_id => plan) } | ||
| scope :of_metric, ->(metric) { where(:metric_id => metric) } |
There was a problem hiding this comment.
Why removing to_param?
Is this being used from somewhere else? no breaking changes?
| @@ -128,6 +128,32 @@ def test_path_field_must_be_a_path | |||
| assert_not_includes accessible_backend_api_config_ids, backend_api_configs[0].id | |||
| end | |||
|
|
|||
| test 'deleting backend usage destroys all related usage limits' do | |||
There was a problem hiding this comment.
This test doesn't do the same thing as in the verification points, but I guess it's valid too.
| puts "Found #{count} orphan usage limits in the database" | ||
|
|
||
| if count.positive? | ||
| orphan_usage_limits.destroy_all if count.positive? |
There was a problem hiding this comment.
Is this not redundant? If we reach here is because count is positive
| orphan_usage_limits.destroy_all if count.positive? | |
| orphan_usage_limits.destroy_all |
There was a problem hiding this comment.
Oh yes, thanks.
It's a result of deciding to add a log entry later.
| # frozen_string_literal: true | ||
|
|
||
| namespace :usage_limits do | ||
| desc 'Deletes instances of UsageLimit for metrics of a backend, that is not linked to the product of the plan anymore' |
There was a problem hiding this comment.
Do we add tests for rake tasks? If so, you could add a test, very similar to the one you already wrote.
There was a problem hiding this comment.
Some rake tasks do have tests, but definitely not all.
I certainly could have written one, but I was a bit lazy, given that this task is probably going to be a one-off, executed just one time. Not sure if it's a valid excuse though 😬
What this PR does / why we need it:
When a backend usage is removed from an API Product, the usage limits that were previously created in this product's plans, associated to the backend API, still remain in the database. They are visible in the plan, but can't be deleted via API.
This PR prevents this situation by deleting all such usage limits from all service's application plans before removing the backend API usage from the service.
TODO:
Which issue(s) this PR fixes
https://issues.redhat.com/browse/THREESCALE-11631
Verification steps
At this point, check the
usage_limitstable in the database (or with an ActiveRecord query), note the ID of the usage limit that is associated to the metric of backend B (let's call this usage limit UL1).On
master(reproducing the issue):4. Unlink backend B from the product
5. Check the
usage_limitstable, and see that the usage limit UL1 is still there.6. Check the application plan, and note that you cannot edit backend B's usage limits anymore
7. Link backend B to the product again
8. Check the application plan, and see that UL1 is "restored"
9. Unlink backend B from the product again
On
backend-level-plan-limitsbranch (verify the rake task):10. Run
bundle exec rake usage_limits:clean_orphans11. Verify in the
usage_limitstable that UL1 is goneOn
backend-level-plan-limitsbranch (verify the fix):12. Link backend 2 to the product again
13. Create a usage limit for a metric of backend 2 again - let's call it UL2
14. Unlink backend B from the product
15. Verify in the
usage_limitstable that UL2 is goneNOTE: at all steps after 3, the usage limits of the metrics of backend A should remain untouched.
Special notes for your reviewer:
