Retrieve the spam params object from the request context
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:
- Get spam_params object from the RequestContext
- changes to the Issues and Snippets Create and Update Services to accommodate the parameter change from
spam_paramstoperform_spam_check, along with the logic to perform spam check or not - changes to
Spam::SpamActionServiceto retrievespam_paramsfrom theGitlab::RequestContextinstead of being passed as parameter - changes to the
UserAgentDetailServiceto receiveperform_spam_checkinstead ofspam_paramsand instead retrieve thespam_paramsfrom theGitlab::RequestContext
- changes to the Issues and Snippets Create and Update Services to accommodate the parameter change from
- 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_paramswere passed and passperform_spam_checkor not
- changes to all calls of the Issues and Snippets Create and Update Services where the
- Remove spam_params parameter from service calls in tests
- same as above, but in the tests
- 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
- Reload the RequestContext when changing the headers mid-request
- when
g-recaptcha-responseorspam_log_idparameters are passed when updating or creating an issue or a snippet, reset the spam params object on theGitlab::RequestContext, since the spam param object was created from headers at the start of the request and theconvert_html_spam_params_to_headersmethod changes the headers during the request.
- when
- 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.
-
I have evaluated the MR acceptance checklist for this MR.