Skip to content

fix(config): correct aggregator type assertion in rules/synonyms export - #283

Open
Hami0095 wants to merge 1 commit into
algolia:mainfrom
Hami0095:fix/config-export-rules-synonyms-aggregator
Open

Hami0095 wants to merge 1 commit into
algolia:mainfrom
Hami0095:fix/config-export-rules-synonyms-aggregator

Conversation

@Hami0095

Copy link
Copy Markdown

Summary

algolia config export silently drops every rule and synonym, and a scope-restricted export of only rules or synonyms fails with No config to export.

In pkg/cmd/shared/config/config.go, the aggregator callbacks passed to BrowseSynonyms/BrowseRules assert the result to a value type:

search.WithAggregator(func(res any, _ error) {
    response, _ := res.(search.SearchSynonymsResponse) // never ok
    synonyms = append(synonyms, response.Hits...)
}),

But the SDK's CreateIterable[T] (in algoliasearch-client-go/v4/algolia/search/api_search.go) always invokes the aggregator with *T:

conf.aggregator(response, responseErr) // response is *T

So the comma-ok assertion always fails, response stays the zero value, and nothing is ever appended — with no error surfaced anywhere, since the assertion failure is discarded.

Every other aggregator in this codebase already asserts to the pointer type (synonyms/browse, rules/browse, objects/browse, indices/analyze), so config.go was the one outlier.

Fix

Assert to *search.SearchSynonymsResponse / *search.SearchRulesResponse, matching the rest of the codebase, with a nil/ok guard.

Test plan

  • Added pkg/cmd/shared/config/config_test.go covering GetSynonyms, GetRules, and GetIndexConfig (previously zero test coverage in this package)
  • Confirmed the new tests fail against unpatched main (proving they actually catch the bug) and pass with the fix
  • go build ./... and go vet ./... clean
  • go test ./... — no regressions in any package touched by this change (pre-existing unrelated Windows-path test failures in a few import packages are untouched by this diff)

BrowseSynonyms and BrowseRules pass a *T to the WithAggregator callback
(see CreateIterable in the SDK), but GetSynonyms/GetRules asserted to
the value type. The comma-ok assertion silently failed on every page,
so rules and synonyms were always dropped from `algolia config export`,
and scoped exports of only rules/synonyms failed with "No config to
export".

Every other aggregator callback in this codebase (synonyms/browse,
rules/browse, objects/browse, indices/analyze) already asserts to the
pointer type; this brings config.go in line with that pattern.

Adds unit tests for GetSynonyms, GetRules, and GetIndexConfig, which
previously had no test coverage.
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 11 complexity · 0 duplication

Metric Results
Complexity 11
Duplication 0

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.

1 participant