Skip to content

Commit

Permalink
Revert "fix: improved elastic search query filter (#837)"
Browse files Browse the repository at this point in the history
This reverts commit b267738.
  • Loading branch information
Junjiequan authored Nov 1, 2023
1 parent b267738 commit 0aca683
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 92 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}

Expand All @@ -42,7 +42,7 @@ jobs:
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}
- name: Cache node_modules
Expand All @@ -64,7 +64,7 @@ jobs:
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}

Expand All @@ -87,7 +87,7 @@ jobs:
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}

Expand Down Expand Up @@ -116,7 +116,7 @@ jobs:
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}

Expand Down Expand Up @@ -179,7 +179,7 @@ jobs:
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}

Expand Down
13 changes: 5 additions & 8 deletions src/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,6 @@ export const createFullfacetPipeline = <T, Y extends object>(
fields: Y,
facets: string[],
subField = "",
esEnabled = false,
): PipelineStage[] => {
const pipeline: PipelineStage[] = [];
const facetMatch: Record<string, unknown> = {};
Expand All @@ -622,13 +621,6 @@ export const createFullfacetPipeline = <T, Y extends object>(
facetMatch[key] = searchExpression<T>(model, key, fields[key as keyof Y]);
}

if (esEnabled) {
if (key === "mode") {
pipelineHandler.handleModeSearch(pipeline, fields, key, idField);
}
return;
}

switch (key) {
case "text":
pipelineHandler.handleTextSearch(pipeline, model, fields, key);
Expand Down Expand Up @@ -914,6 +906,11 @@ const replaceLikeOperatorRecursive = (
return output;
};

export const isObjectWithOneKey = (obj: object): boolean => {
const keys = Object.keys(obj);
return keys.length === 1;
};

export const sleep = (ms: number) => {
return new Promise((resolve) => setTimeout(resolve, ms));
};
47 changes: 16 additions & 31 deletions src/datasets/datasets.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
createFullfacetPipeline,
createFullqueryFilter,
extractMetadataKeys,
isObjectWithOneKey,
parseLimitFilters,
} from "src/common/utils";
import { ElasticSearchService } from "src/elastic-search/elastic-search.service";
Expand Down Expand Up @@ -108,12 +109,7 @@ export class DatasetsService {
};
const modifiers: QueryOptions = parseLimitFilters(filter.limits);

const isFieldsEmpty = Object.keys(whereClause).length === 0;

// NOTE: if Elastic search DB is empty we should use default mongo query
const canPerformElasticSearchQueries = await this.isElasticSearchDBEmpty();

if (!this.ESClient || isFieldsEmpty || !canPerformElasticSearchQueries) {
if (!this.ESClient || !whereClause) {
datasets = await this.datasetModel
.find(whereClause, null, modifiers)
.exec();
Expand All @@ -123,8 +119,9 @@ export class DatasetsService {
modifiers.limit,
modifiers.skip,
);

datasets = await this.datasetModel
.find({ _id: { $in: esResult.data } })
.find({ _id: { $in: esResult.data } }, null, modifiers)
.exec();
}

Expand All @@ -134,19 +131,23 @@ export class DatasetsService {
async fullFacet(
filters: IFacets<IDatasetFields>,
): Promise<Record<string, unknown>[]> {
let data;

const fields = filters.fields ?? {};
const facets = filters.facets ?? [];

// NOTE: if fields contains no value, we should use mongo query to optimize performance.
// however, fields always contain "mode" key, so we need to check if there's more than one key
const isFieldsEmpty = Object.keys(fields).length === 1;
// however, fields always contain mode key, so we need to check if there's more than one key
const isFieldsEmpty = isObjectWithOneKey(fields);

// NOTE: if Elastic search DB is empty we should use default mongo query
const canPerformElasticSearchQueries = await this.isElasticSearchDBEmpty();
let data;
if (this.ESClient && !isFieldsEmpty) {
const totalDocCount = await this.datasetModel.countDocuments();

const { data: esPids } = await this.ESClient.search(
fields as IDatasetFields,
totalDocCount,
);

if (!this.ESClient || isFieldsEmpty || !canPerformElasticSearchQueries) {
fields.mode = { _id: { $in: esPids } };
const pipeline = createFullfacetPipeline<DatasetDocument, IDatasetFields>(
this.datasetModel,
"pid",
Expand All @@ -157,25 +158,15 @@ export class DatasetsService {

data = await this.datasetModel.aggregate(pipeline).exec();
} else {
const { count: initialCount } = await this.ESClient.getCount();
const { totalCount, data: esPids } = await this.ESClient.search(
fields as IDatasetFields,
initialCount,
);

fields.mode = { _id: { $in: esPids } };
const pipeline = createFullfacetPipeline<DatasetDocument, IDatasetFields>(
this.datasetModel,
"pid",
fields,
facets,
"",
!!this.ESClient,
);
data = await this.datasetModel.aggregate(pipeline).exec();

// NOTE: below code is to overwrite totalCount with ES result
data[0].all = [{ totalSets: totalCount }];
data = await this.datasetModel.aggregate(pipeline).exec();
}

return data;
Expand Down Expand Up @@ -497,10 +488,4 @@ export class DatasetsService {
}
}
}

async isElasticSearchDBEmpty() {
if (!this.ESClient) return;
const count = await this.ESClient.getCount();
return count.count > 0;
}
}
10 changes: 6 additions & 4 deletions src/elastic-search/configuration/indexSetting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ export const special_character_filter: AnalysisPatternReplaceCharFilter = {
};

//Dynamic templates
export const dynamic_template: Record<string, MappingDynamicTemplate>[] = [
export const dynamic_template:
| Record<string, MappingDynamicTemplate>[]
| never = [
{
string_as_keyword: {
path_match: "scientificMetadata.*.*",
Expand All @@ -33,16 +35,16 @@ export const dynamic_template: Record<string, MappingDynamicTemplate>[] = [
},
},
{
long_as_double: {
long_as_long: {
path_match: "scientificMetadata.*.*",
match_mapping_type: "long",
mapping: {
type: "double",
coerce: true,
type: "long",
ignore_malformed: true,
},
},
},

{
date_as_keyword: {
path_match: "scientificMetadata.*.*",
Expand Down
15 changes: 1 addition & 14 deletions src/elastic-search/elastic-search.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,18 +176,6 @@ export class ElasticSearchService implements OnModuleInit {
return bulkResponse;
}

async getCount(index = this.defaultIndex) {
try {
return await this.esService.count({ index });
} catch (error) {
Logger.error("getCount failed-> ElasticSearchService", error);
throw new HttpException(
`getCount failed-> ElasticSearchService ${error}`,
HttpStatus.BAD_REQUEST,
);
}
}

async updateIndex(index = this.defaultIndex) {
try {
await this.esService.indices.close({
Expand Down Expand Up @@ -259,8 +247,7 @@ export class ElasticSearchService implements OnModuleInit {
const searchOptions = {
track_scores: true,
body: searchQuery,
from: skip,
size: limit,
size: limit + skip,
sort: [{ _score: { order: "desc" } }],
min_score: defaultMinScore,
track_total_hits: true,
Expand Down
12 changes: 3 additions & 9 deletions src/elastic-search/helpers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,11 @@ export const convertToElasticSearchQuery = (
for (const field in scientificQuery) {
const query = scientificQuery[field] as Record<string, unknown>;
const operation = Object.keys(query)[0];
const value =
typeof query[operation] === "string"
? (query[operation] as string).trim()
: query[operation];

const value = query[operation];
const esOperation = operation.replace("$", "");

// NOTE-EXAMPLE:
// trasnformedKey = "scientificMetadata.someKey.value"
// firstPart = "scientificMetadata",
// middlePart = "someKey"
// Example: trasnformedKey = "scientificMetadata.someKey.value"
// firstPart = "scientificMetadata" , middlePart = "someKey"
const { transformedKey, firstPart, middlePart } = transformMiddleKey(field);

let filter = {};
Expand Down
12 changes: 1 addition & 11 deletions src/elastic-search/interfaces/es-common.type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,6 @@ export interface IShould {
};
}

export interface IBoolShould {
bool: {
should: IShould[];
minimum_should_match?: number;
};
}

export interface IFilter {
terms?: {
[key: string]: string[];
Expand All @@ -42,15 +35,12 @@ export interface IFilter {
lte?: string | number;
};
};
match?: {
[key: string]: string | number;
};
nested?: {
path: string;
query: {
bool: {
must: (
| { term?: { [key: string]: string } }
| { match?: { [key: string]: string } }
| { range?: { [key: string]: { [key: string]: string | number } } }
)[];
};
Expand Down
19 changes: 10 additions & 9 deletions src/elastic-search/providers/query-builder.service.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import { Injectable, Logger } from "@nestjs/common";
import { QueryDslQueryContainer } from "@elastic/elasticsearch/lib/api/types";
import { IDatasetFields } from "src/datasets/interfaces/dataset-filters.interface";
import {
IBoolShould,
IFilter,
IShould,
ObjectType,
} from "../interfaces/es-common.type";
import { IFilter, IShould, ObjectType } from "../interfaces/es-common.type";
import { FilterFields, QueryFields } from "./fields.enum";
import { mapScientificQuery } from "src/common/utils";
import { IScientificFilter } from "src/common/interfaces/common.interface";
Expand Down Expand Up @@ -62,7 +57,7 @@ export class SearchQueryService {

shouldFilter.push(ownerGroup, accessGroups);
}
return { bool: { should: shouldFilter, minimum_should_match: 1 } };
return shouldFilter;
}

private buildTextQuery(text: string): QueryDslQueryContainer[] {
Expand Down Expand Up @@ -166,13 +161,19 @@ export class SearchQueryService {

private constructFinalQuery(
filter: IFilter[],
should: IBoolShould,
should: IShould[],
query: QueryDslQueryContainer[],
) {
const finalQuery = {
query: {
bool: {
filter: [...filter, should],
filter,
should: {
bool: {
should,
minimum_should_match: 1,
},
},
must: query,
},
},
Expand Down

0 comments on commit 0aca683

Please sign in to comment.