Use Gitlab::Json.safe_parse in BaseBatchBulkUpdateService

What does this MR do?

Replaces Gitlab::Json.parse with Gitlab::Json::SafeParser.parse in the load_cursor private method of ee/app/services/geo/base_batch_bulk_update_service.rb. This fixes a Gitlab/JsonSafeParse RuboCop violation. SafeParser.parse adds built-in protections against excessively large JSON payloads and deeply nested JSON structures before delegating to the underlying parser, making the behavior identical for all valid cursor values used by this service. The spec file was also updated to address a second round of maintainer review feedback: adding the :clean_gitlab_redis_shared_state tag to prevent Redis state leakage between tests, renaming and updating the cursor-parse assertion to match SafeParser.parse, deduplicating cursor setup logic in the 'with cursor continuation' context, and documenting the current error-handling behavior for invalid JSON cursors.

Why was this MR needed?

The Gitlab/JsonSafeParse RuboCop cop (introduced in MR !215073 (merged) (merged)) flags any use of Gitlab::Json.parse that should instead use Gitlab::Json::SafeParser.parse when no additional parse options are needed. The load_cursor method reads a JSON-encoded cursor from Redis and was using the unchecked parse variant. The cursor value is always a small integer or array of integers written internally by the same service via Gitlab::Json.dump, so it will never exceed SafeParser.parse's limits — the switch is safe with no functional impact.

What are the relevant issue numbers?

Closes #586122 (closed)

Related to #600071 (follow-up to make load_cursor recover from a corrupted cursor instead of failing every retry)

Screenshots / Recordings (if applicable)

N/A

Does this MR meet the acceptance criteria?

[x] Tests added/changed for new behavior — in ee/spec/services/geo/bulk_primary_verification_service_spec.rb:

  1. Added the :clean_gitlab_redis_shared_state tag to the top-level describe to prevent Redis state leakage between examples
  2. Renamed the parse assertion to it 'parses the cursor using SafeParser' and updated the stub to Gitlab::Json::SafeParser/:parse
  3. Simplified the with cursor continuation before block to reuse the existing serialized let instead of redundantly recomputing it
  4. Added it 'raises JSON::ParserError' in a when cursor contains invalid JSON context, with a comment documenting current behavior and linking the follow-up issue (#600071)
  5. Removed the describe '#load_cursor' block from ee/spec/services/geo/base_batch_bulk_update_service_spec.rb that was introduced in the first commit, since that coverage now lives in the integration-level spec above

[x] Removed ee/app/services/geo/base_batch_bulk_update_service.rb from .rubocop_todo/gitlab/json_safe_parse.yml since the service now calls Gitlab::Json::SafeParser.parse directly

[x] All tests passing — every example in bulk_primary_verification_service_spec.rb (28 examples, plus 44 from the with different model types table) run and passing

[x] Follows project style guide — bundle exec rubocop on both changed files reports 2 files inspected, no offenses detected; commit message passed lint_commit_message hook

[x] No breaking changes introduced — SafeParser.parse performs the same validation as safe_parse before delegating to the underlying parser; return value is identical for all valid cursor data

Edited by Ryan Lawton

Merge request reports

Loading
Loading