get back the key collision check
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
@@ -236,5 +236,57 @@ describe('buildEntitySearch', () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('rejects duplicate keys', () => {
|
||||
expect(() =>
|
||||
buildEntitySearch('eid', {
|
||||
relations: [],
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
metadata: {
|
||||
name: 'n',
|
||||
Namespace: 'ns',
|
||||
},
|
||||
}),
|
||||
).toThrow(
|
||||
`Entity has duplicate keys that vary only in casing, 'metadata.namespace'`,
|
||||
);
|
||||
|
||||
expect(() =>
|
||||
buildEntitySearch('eid', {
|
||||
relations: [
|
||||
{
|
||||
type: 'dup',
|
||||
targetRef: 'k:ns/a',
|
||||
},
|
||||
{
|
||||
type: 'DUP',
|
||||
targetRef: 'k:ns/b',
|
||||
},
|
||||
],
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
metadata: { name: 'n' },
|
||||
}),
|
||||
).toThrow(
|
||||
`Entity has duplicate keys that vary only in casing, 'relations.dup'`,
|
||||
);
|
||||
|
||||
expect(() =>
|
||||
buildEntitySearch('eid', {
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
metadata: { name: 'n' },
|
||||
spec: {
|
||||
owner: 'o',
|
||||
OWNER: 'o',
|
||||
lifecycle: 'production',
|
||||
lifeCycle: 'production',
|
||||
},
|
||||
}),
|
||||
).toThrow(
|
||||
`Entity has duplicate keys that vary only in casing, 'spec.owner', 'spec.lifecycle'`,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
*/
|
||||
|
||||
import { DEFAULT_NAMESPACE, Entity } from '@backstage/catalog-model';
|
||||
import { InputError } from '@backstage/errors';
|
||||
import { DbSearchRow } from '../../tables';
|
||||
|
||||
// These are excluded in the generic loop, either because they do not make sense
|
||||
@@ -199,5 +200,23 @@ export function buildEntitySearch(
|
||||
});
|
||||
}
|
||||
|
||||
// This validates that there are no keys that vary only in casing, such
|
||||
// as `spec.foo` and `spec.Foo`.
|
||||
const keys = new Set(raw.map(r => r.key));
|
||||
const lowerKeys = new Set(raw.map(r => r.key.toLocaleLowerCase('en-US')));
|
||||
if (keys.size !== lowerKeys.size) {
|
||||
const difference = [];
|
||||
for (const key of keys) {
|
||||
const lower = key.toLocaleLowerCase('en-US');
|
||||
if (!lowerKeys.delete(lower)) {
|
||||
difference.push(lower);
|
||||
}
|
||||
}
|
||||
const badKeys = `'${difference.join("', '")}'`;
|
||||
throw new InputError(
|
||||
`Entity has duplicate keys that vary only in casing, ${badKeys}`,
|
||||
);
|
||||
}
|
||||
|
||||
return mapToRows(raw, entityId);
|
||||
}
|
||||
|
||||
@@ -209,6 +209,11 @@ export async function performStitching(options: {
|
||||
entity.metadata.etag = hash;
|
||||
}
|
||||
|
||||
// This may throw if the entity is invalid, so we call it before
|
||||
// the final_entities write, even though we may end up not needing
|
||||
// to write the search index.
|
||||
const searchEntries = buildEntitySearch(entityId, entity);
|
||||
|
||||
const amountOfRowsChanged = await knex<DbFinalEntitiesRow>('final_entities')
|
||||
.update({
|
||||
final_entity: JSON.stringify(entity),
|
||||
@@ -223,7 +228,6 @@ export async function performStitching(options: {
|
||||
return 'abandoned';
|
||||
}
|
||||
|
||||
const searchEntries = buildEntitySearch(entityId, entity);
|
||||
await knex.transaction(async trx => {
|
||||
await trx<DbSearchRow>('search').where({ entity_id: entityId }).delete();
|
||||
await trx.batchInsert('search', searchEntries, BATCH_SIZE);
|
||||
|
||||
Reference in New Issue
Block a user