Skip to content

fix(import): apply clear/replace existing on the final batch - #285

Open
Hami0095 wants to merge 1 commit into
algolia:mainfrom
Hami0095:fix/import-clear-existing-tail-batch
Open

Hami0095 wants to merge 1 commit into
algolia:mainfrom
Hami0095:fix/import-clear-existing-tail-batch

Conversation

@Hami0095

Copy link
Copy Markdown

Summary

Fixes #191.

algolia rules import --clear-existing-rules (and algolia synonyms import --replace-existing-synonyms) silently does nothing for any input file under 1000 entries — which is nearly all of them.

Both commands import in batches of 1000. In pkg/cmd/rules/import/import.go, the clear flag is only attached to the SaveRules request in the batch-full path:

if count == batchSize {
    res, err := client.SaveRules(
        client.NewApiSaveRulesRequest(opts.Index, rules).
            WithClearExistingRules(clearExistingRules).
            WithForwardToReplicas(opts.ForwardToReplicas),
    )
    ...

The final partial batch calls SaveRules with no clear flag at all:

if count > 0 {
    res, err := client.SaveRules(
        client.NewApiSaveRulesRequest(opts.Index, rules).
            WithForwardToReplicas(opts.ForwardToReplicas), // no WithClearExistingRules
    )

A file under 1000 rules never fills the buffer, so this final call is the only SaveRules request sent — and it drops the flag. The existing ClearRules fallback is guarded by totalCount == 0, so it only helps for an empty input file, not a small non-empty one.

pkg/cmd/synonyms/import/import.go has the identical defect: WithReplaceExistingSynonyms is attached in the batch-full path but not the tail block.

Fix

Pass the flag to the final batch's request too, conditioned on whether it hasn't already been consumed by an earlier full batch — this keeps the wire format unchanged for multi-batch imports (a later request still omits the parameter rather than sending clearExistingRules=false).

Test plan

  • Added a sub-batch-size case to both pkg/cmd/rules/import/import_test.go and pkg/cmd/synonyms/import/import_test.go asserting the outgoing request carries the flag when set, and omits it when not
  • Confirmed the new "small batch ... clear existing" cases fail against unpatched main and pass with the fix
  • go build ./..., go vet ./... clean
  • go test ./... — no new regressions (pre-existing Windows-path test failures in a handful of import/operations/update packages are unchanged by this diff, and unrelated to it)

🤖 Generated with Claude Code

Rules and synonyms are imported in batches of 1000. The clear flag was
attached only to the request sent when the buffer filled, so the final
partial batch was saved without it.

For any file holding fewer than 1000 entries there is no full batch at
all, so the only request sent carried no flag and
--clear-existing-rules / --replace-existing-synonyms did nothing. The
existing ClearRules/ClearSynonyms fallback does not cover this: it is
guarded by totalCount == 0, so it only fires for an empty input file.

The flag is attached only when it is still set, so a multi-batch
import's later requests keep omitting the parameter rather than
sending clearExistingRules=false, preserving the current wire format.

Fixes algolia#191

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 10 duplication

Metric Results
Duplication 10

View in Codacy

TIP This summary will be updated as you push new changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

algolia rules import

1 participant