Skip to content

Commit

Permalink
address platform PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
legrego committed Jul 13, 2020
1 parent 6c2df02 commit 7a095e8
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -385,4 +385,18 @@ describe('#getQueryParams', () => {
});
});
});

describe('namespaces property', () => {
ALL_TYPES.forEach((type) => {
it(`throws for ${type} when namespaces is an empty array`, () => {
expect(() =>
getQueryParams({
mappings,
registry,
namespaces: [],
})
).toThrowError('cannot specify empty namespaces array');
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ function getClauseForType(
namespaces: string[] = ['default'],
type: string
) {
if (namespaces.length === 0) {
throw new Error('cannot specify empty namespaces array');
}
if (registry.isMultiNamespace(type)) {
return {
bool: {
Expand All @@ -83,7 +86,8 @@ function getClauseForType(
should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } });
}
if (should.length === 0) {
throw new Error('cannot specify empty namespaces array');
// This is indicitive of a bug, and not user error.
throw new Error('unhandled search condition: expected at least 1 `should` clause.');
}
return {
bool: {
Expand Down Expand Up @@ -136,6 +140,15 @@ export function getQueryParams({
}: QueryParams) {
const types = getTypes(mappings, type);

// A de-duplicated set of namespaces makes for a more effecient query.
//
// Additonally, we treat the `*` namespace as the `default` namespace.
// In the Default Distribution, the `*` is automatically expanded to include all available namespaces.
// However, the OSS distribution (and certain configurations of the Default Distribution) can allow the `*`
// to pass through to the SO Repository, and eventually to this module. When this happens, we translate to `default`,
// since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place
// would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard.
// We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716
const normalizedNamespaces = namespaces
? Array.from(
new Set(namespaces.map((namespace) => (namespace === '*' ? 'default' : namespace)))
Expand Down

0 comments on commit 7a095e8

Please sign in to comment.