fix(catalog-backend): use explicit null sentinel in dedup keys and add buildEntitySearch dedup tests
Use `=== null ? '\x01' : value` instead of `?? ''` in both buildEntitySearch and syncSearchRows dedup maps, so that null and empty-string values are treated as distinct keys. In theory an entity could produce both value=null and value='' for the same key (e.g. spec.foo: [null, '']), and the old encoding would silently drop one of the two distinct rows. Also adds two focused unit tests to buildEntitySearch.test.ts: one covering deduplication of duplicate array values (e.g. tags: ['java', 'java', 'Java']), and one confirming that null and empty-string are kept as separate rows. Signed-off-by: Fredrik Adelöw <freben@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -255,6 +255,41 @@ describe('buildEntitySearch', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('deduplicates rows from duplicate array values', () => {
|
||||
const rows = buildEntitySearch('eid', {
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
metadata: { name: 'n', tags: ['java', 'java', 'Java'] },
|
||||
});
|
||||
// 'java' and 'Java' both normalise to value 'java'; all three occurrences
|
||||
// should collapse into a single row (first-wins for original_value).
|
||||
const tagRows = rows.filter(r => r.key === 'metadata.tags');
|
||||
expect(tagRows).toHaveLength(1);
|
||||
expect(tagRows[0]).toEqual(
|
||||
expect.objectContaining({ value: 'java', original_value: 'java' }),
|
||||
);
|
||||
// The synthetic boolean path key (metadata.tags.java) should also appear
|
||||
// exactly once.
|
||||
const boolRows = rows.filter(r => r.key === 'metadata.tags.java');
|
||||
expect(boolRows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('treats null and empty-string values as distinct for deduplication', () => {
|
||||
// An array with both null and '' for the same key must produce two rows
|
||||
// since value=null and value='' are distinct in the database.
|
||||
const rows = buildEntitySearch('eid', {
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
metadata: { name: 'n' },
|
||||
spec: { foo: [null, ''] },
|
||||
});
|
||||
const fooRows = rows.filter(r => r.key === 'spec.foo');
|
||||
expect(fooRows).toHaveLength(2);
|
||||
const values = fooRows.map(r => r.value);
|
||||
expect(values).toContain(null);
|
||||
expect(values).toContain('');
|
||||
});
|
||||
|
||||
it('rejects duplicate keys', () => {
|
||||
expect(() =>
|
||||
buildEntitySearch('eid', {
|
||||
|
||||
@@ -235,7 +235,7 @@ export function buildEntitySearch(
|
||||
// violate the unique constraint on (entity_id, key, value).
|
||||
const seen = new Set<string>();
|
||||
return rows.filter(row => {
|
||||
const k = `${row.key}\0${row.value ?? ''}`;
|
||||
const k = `${row.key}\0${row.value === null ? '\x01' : row.value}`;
|
||||
if (seen.has(k)) return false;
|
||||
seen.add(k);
|
||||
return true;
|
||||
|
||||
@@ -50,11 +50,15 @@ export async function syncSearchRows(
|
||||
): Promise<void> {
|
||||
// Dedup by (key, value) — the UNIQUE constraint on (entity_id, key, value)
|
||||
// rejects duplicates, and the same lowercased value with different original
|
||||
// casing is semantically a single entry. Keep the last occurrence so that
|
||||
// if the entity data has e.g. tags: ['Java', 'java'], the later one wins.
|
||||
// casing is semantically a single entry. Keep the first occurrence, which
|
||||
// matches the first-wins semantics of buildEntitySearch so that both layers
|
||||
// consistently pick the same original_value for a given input order.
|
||||
const dedupMap = new Map<string, DbSearchRow>();
|
||||
for (const entry of searchEntries) {
|
||||
dedupMap.set(`${entry.key}\0${entry.value ?? ''}`, entry);
|
||||
const k = `${entry.key}\0${entry.value === null ? '\x01' : entry.value}`;
|
||||
if (!dedupMap.has(k)) {
|
||||
dedupMap.set(k, entry);
|
||||
}
|
||||
}
|
||||
const deduped = [...dedupMap.values()];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user