Skip to content

Retrieve the spam params object from the request context

Alex Buijs requested to merge refactor-spam-params into master

What does this MR do and why?

This MR refactors how spam_params are passed to the Spam::SpamActionService.

Before this refactor, the spam_params were set in each place where the Issues::CreateService, Issues::UpdateService, Snippets::CreateService or the Snippets::UpdateService was (indirectly) called.

After this refactor, the spam_params are only set once, inside the Spam::SpamActionService. This was made possible by this recently merged MR, which added the spam_params to the Gitlab::RequestContext.

Because the passing of the spam_params was not only used to pass along an object, but also to control spam check (by disabling it when set to nil for the Create Services and by enabling when set for the Update Services), we now added the perform_spam_check parameter. It is set by default to true in the Create Services and to false in the Update Services. To implement this, we changed the parameters in all the service calls like this:

service before after
Issues::CreateService / Snippets::CreateService spam_params: nil perform_spam_check: false
Issues::CreateService / Snippets::CreateService spam_params: spam_params no added param
Issues::UpdateService / Snippets::UpdateService spam_params: spam_params perform_spam_check: true

Each commit contains one logical change, to ease reviewing:

  1. Get spam_params object from the RequestContext
    • changes to the Issues and Snippets Create and Update Services to accommodate the parameter change from spam_params to perform_spam_check, along with the logic to perform spam check or not
    • changes to Spam::SpamActionService to retrieve spam_params from the Gitlab::RequestContext instead of being passed as parameter
    • changes to the UserAgentDetailService to receive perform_spam_check instead of spam_params and instead retrieve the spam_params from the Gitlab::RequestContext
  2. Refactor all service calls to pass perform_spam_check parameter
    • changes to all calls of the Issues and Snippets Create and Update Services where the spam_params were passed and pass perform_spam_check or not
  3. Remove spam_params parameter from service calls in tests
    • same as above, but in the tests
  4. Remove stub_spam_services from tests
    • this stub is no longer needed, as we do not create a spam params object from the request anymore every time we call one of the services
  5. Reload the RequestContext when changing the headers mid-request
    • when g-recaptcha-response or spam_log_id parameters are passed when updating or creating an issue or a snippet, reset the spam params object on the Gitlab::RequestContext, since the spam param object was created from headers at the start of the request and the convert_html_spam_params_to_headers method changes the headers during the request.
  6. Fix issues controller spec
    • this contains some fixes to the issues controller spec

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Alex Buijs

Merge request reports