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 Backend API mapping rules injected into the proxy config #1193

Merged
merged 3 commits into from
Sep 17, 2019

Conversation

guicassolato
Copy link
Contributor

@guicassolato guicassolato commented Sep 16, 2019

Which issue(s) this PR fixes

Backend api mapping rules injected in the proxy config have two problems:

  1. It does not prepend the backend API config path to the pattern of the proxy rule
  2. It does not inject the full system_name of the metric but only the part without the .#{backend_api.id} suffix

First problem is due to draper's delegate_all method used in

. This method makes the decorator class to dynamically define delegations of the decorated object's instance methods to the object itself, thus making ProxyRuleDecorator#pattern to execute only once and then always be delegated to the object.

See drapergem/draper#818

The second problem is because of

def system_name
attributes['system_name'].to_s.gsub(/#{Regexp.escape(SYSTEM_NAME_SUFFIX_SEPARATOR)}\d+\z/, '')
end

What this PR does

  • Fix path of Backend API mapping rules injected into the proxy config
  • Fix system_name of Backend API mapping rules injected into the proxy config

Verification steps

  1. Generate a json proxy config file of a product using 2+ backend APIs containing proxy rules
  2. Verify that all backend API proxy rules include the proper backend api config's path prepended to the rule's pattern
  3. Verify that all backend API proxy rules have the .#{backend_api.id} suffix appended to the end of metric_system_name

Screen Shot 2019-09-16 at 8 18 51 PM

@guicassolato guicassolato self-assigned this Sep 16, 2019
Copy link
Contributor

@hallelujah hallelujah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:
Draper has a bug with collections drapergem/draper#818
Thank you!

@hallelujah hallelujah merged commit 4a31c49 into master Sep 17, 2019
@hallelujah hallelujah deleted the fix/injection_backend_api_proxy_rules branch September 17, 2019 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants