Skip to content

Replace Gitlab::Elastic::Helper.refresh_index

What does this MR do?

Replaces all calls to Gitlab::Elastic::Helpers.refresh_index in the specs with a new spec helper method: ensure_elasticsearch_index!

This is important preparatory work for !24298 (merged) , where an alternative mechanism for performing indexing will be introduced. Instead of enqueuing sidekiq jobs (which are automatically run by rspec), we'll be pushing information to a Redis ZSET, which is asynchronously consumed by a cron worker that does not run in the specs.

Extracting out these calls to refresh_index gives me a natural point to implement a hook to run that cron worker in specs, processing the update messages. This should be enough to make the specs pass with minimal changes in the other MR.

Another option in that MR would be to modify the call to ::Elastic::ProcessBookkeepingService.track! in specs, such that the tracking is done synchronously, rather than asynchronously. At the moment I prefer the depth of testing that this approach offers, but I'm definitely open to discussion about it!

Since the refresh_index call is only used in the specs, we might decide that this change is good and useful even if we decide to go the synchronous route in the other MR. It would allow us to remove the methods_for_all_write_targets abstraction in Elastic::ClassProxyUtil, for instance.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #34086 (closed)

Edited by Nick Thomas

Merge request reports

Loading