Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(contentful): add paging in searchByKeywords #1740

Merged
merged 7 commits into from
Jul 28, 2021
Merged
175 changes: 90 additions & 85 deletions packages/botonic-plugin-contentful/package-lock.json

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions packages/botonic-plugin-contentful/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"postversion": "git push && git push --tags"
},
"name": "@botonic/plugin-contentful",
"version": "0.19.0-alpha.5",
"version": "0.19.0-alpha.6",
"license": "MIT",
"main": "lib/index.js",
"types": "lib/index.d.ts",
Expand Down Expand Up @@ -64,7 +64,6 @@
"memoizee": "^0.4.15",
"moment": "^2.29.1",
"moment-timezone": "^0.5.33",
"perf_hooks": "0.0.1",
"sort-stream": "^1.0.1",
"xlsx": "^0.17.0"
},
Expand Down
5 changes: 4 additions & 1 deletion packages/botonic-plugin-contentful/src/cms/cms-dummy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ export class DummyCMS implements CMS {
return Promise.resolve([])
}

contentsWithKeywords({} = DEFAULT_CONTEXT): Promise<SearchCandidate[]> {
contentsWithKeywords(
{} = DEFAULT_CONTEXT,
paging?: PagingOptions
): Promise<SearchCandidate[]> {
const contents = this.buttonCallbacks.map((cb, id) => {
const button = DummyCMS.buttonFromCallback(cb)
// eslint-disable-next-line @typescript-eslint/no-unused-vars
Expand Down
7 changes: 5 additions & 2 deletions packages/botonic-plugin-contentful/src/cms/cms-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,12 @@ export class ErrorReportingCMS implements CMS {
)
}

contentsWithKeywords(context?: Context): Promise<SearchCandidate[]> {
contentsWithKeywords(
context?: Context,
paging?: PagingOptions
): Promise<SearchCandidate[]> {
return this.cms
.contentsWithKeywords(context)
.contentsWithKeywords(context, paging)
.catch(this.handleError('contentsWithKeywords', {}, context))
}

Expand Down
7 changes: 5 additions & 2 deletions packages/botonic-plugin-contentful/src/cms/cms-log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,12 @@ export class LogCMS implements CMS {
return this.cms.content(id, context)
}

contentsWithKeywords(context?: Context): Promise<SearchCandidate[]> {
contentsWithKeywords(
context?: Context,
paging?: PagingOptions
): Promise<SearchCandidate[]> {
this.logger('contentsWithKeywords')
return this.cms.contentsWithKeywords(context)
return this.cms.contentsWithKeywords(context, paging)
}

topContents<T extends TopContent>(
Expand Down
7 changes: 5 additions & 2 deletions packages/botonic-plugin-contentful/src/cms/cms-multilocale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ export class MultiContextCms implements CMS {
return this.cmsFromContext(context).assets(context)
}

contentsWithKeywords(context?: Context): Promise<SearchCandidate[]> {
return this.cmsFromContext(context).contentsWithKeywords(context)
contentsWithKeywords(
context?: Context,
paging?: PagingOptions
): Promise<SearchCandidate[]> {
return this.cmsFromContext(context).contentsWithKeywords(context, paging)
}

dateRange(id: string, context?: Context): Promise<DateRangeContent> {
Expand Down
5 changes: 4 additions & 1 deletion packages/botonic-plugin-contentful/src/cms/cms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ export interface CMS {
* For contents with 'Searchable by' field (eg. {@link Queue}), it returns one result per each 'Seachable by' entry
* @param context If locale specified, it does not return contents without values for the locale (even if it has value for the fallback locale)
*/
contentsWithKeywords(context?: Context): Promise<SearchCandidate[]>
contentsWithKeywords(
context?: Context,
paging?: PagingOptions
): Promise<SearchCandidate[]>
schedule(id: string, context?: Context): Promise<ScheduleContent>
dateRange(id: string, context?: Context): Promise<DateRangeContent>
asset(id: string, context?: Context): Promise<Asset>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,11 @@ export class FilteredCMS implements CMS {
return this.cms.content(id, context)
}

contentsWithKeywords(context?: Context): Promise<SearchCandidate[]> {
return this.cms.contentsWithKeywords(context)
contentsWithKeywords(
context?: Context,
paging?: PagingOptions
): Promise<SearchCandidate[]> {
return this.cms.contentsWithKeywords(context, paging)
}

async topContents<T extends TopContent>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,10 @@ export class Contentful implements cms.CMS {
}

async contentsWithKeywords(
context = DEFAULT_CONTEXT
context = DEFAULT_CONTEXT,
paging = new PagingOptions()
): Promise<SearchCandidate[]> {
return this._keywords.contentsWithKeywords(context)
return this._keywords.contentsWithKeywords(context, paging)
}

async schedule(id: string): Promise<ScheduleContent> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { Entry, EntryCollection } from 'contentful'

import * as cms from '../../cms'
import { CommonFields, Context, TopContentId, TopContentType } from '../../cms'
import { ContentType } from '../../cms/cms'
import {
CommonFields,
ContentType,
Context,
PagingOptions,
TopContentId,
TopContentType,
} from '../../cms'
import { SearchCandidate } from '../../search'
import { QueueFields } from '../contents/queue'
import { DeliveryApi } from '../delivery-api'
Expand All @@ -13,6 +19,7 @@ export class KeywordsDelivery {

async contentsWithKeywords(
context: Context,
paging: PagingOptions,
modelsWithKeywords = [
ContentType.TEXT,
ContentType.CAROUSEL,
Expand All @@ -22,7 +29,11 @@ export class KeywordsDelivery {
): Promise<SearchCandidate[]> {
// TODO maybe it's more efficient to get all contents (since most have keywords anyway and we normally have few non
// TopContents such as Buttons)
const fromKeywords = this.entriesWithKeywords(context, modelsWithKeywords)
const fromKeywords = this.entriesWithKeywords(
context,
modelsWithKeywords,
paging
)
const fromSearchable = this.entriesWithSearchableByKeywords(
context,
modelsWithSearchableByKeywords
Expand Down Expand Up @@ -99,10 +110,12 @@ export class KeywordsDelivery {

private entriesWithKeywords(
context: Context,
models: TopContentType[]
models: TopContentType[],
paging: PagingOptions
): Promise<SearchCandidate[]> {
const getWithKeywords = (contentType: cms.TopContentType) =>
this.delivery.getEntries<CommonEntryFields>(context, {
...paging,
// eslint-disable-next-line @typescript-eslint/naming-convention
content_type: contentType,
'fields.keywords[exists]': true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as cms from '../cms'
import { PagingOptions } from '../cms'
import {
checkLocale,
KeywordsOptions,
Expand All @@ -24,10 +25,14 @@ export class SearchByKeywords {
async searchContentsFromInput(
inputText: NormalizedUtterance,
matchType: MatchType,
context: cms.ContextWithLocale
context: cms.ContextWithLocale,
paging?: PagingOptions
): Promise<SearchResult[]> {
checkLocale(context.locale)
const contentsWithKeywords = await this.cms.contentsWithKeywords(context)
const contentsWithKeywords = await this.cms.contentsWithKeywords(
context,
paging
)
const options =
this.keywordsOptions[context.locale] ||
this.keywordsOptions[languageFromLocale(context.locale)] ||
Expand Down
7 changes: 5 additions & 2 deletions packages/botonic-plugin-contentful/src/search/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ContentType,
Context,
ContextWithLocale,
PagingOptions,
Text,
} from '../cms'
import { checkLocale, KeywordsOptions, MatchType, Normalizer } from '../nlp'
Expand All @@ -28,14 +29,16 @@ export class Search {
async searchByKeywords(
inputText: string,
matchType: MatchType,
context: ContextWithLocale
context: ContextWithLocale,
paging: PagingOptions = new PagingOptions()
): Promise<SearchResult[]> {
const locale = checkLocale(context.locale)
const utterance = this.normalizer.normalize(locale, inputText)
const results = await this.search.searchContentsFromInput(
utterance,
matchType,
context
context,
paging
)
return this.search.filterChitchat(utterance.words, results)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,25 @@ class PerformanceFactory {
try {
return performance
} catch {
// eslint-disable-next-line @typescript-eslint/no-var-requires
return require('perf_hooks').performance
//TODO: investigate why a normal require breaks the bot when compiling with webpack
//To solve the issue we're executing eval('require') instead of using a direct require (https://stackoverflow.com/a/67288823/3982733)
return eval('require')('perf_hooks').performance
}
}
static getObserver(): typeof PerformanceObserver {
try {
return PerformanceObserver
} catch {
// eslint-disable-next-line @typescript-eslint/no-var-requires
return require('perf_hooks').PerformanceObserver
//TODO: investigate why a normal require breaks the bot when compiling with webpack
//To solve the issue we're executing eval('require') instead of using a direct require (https://stackoverflow.com/a/67288823/3982733)
return eval('require')('perf_hooks').PerformanceObserver
}
}
}
export class NodePerformanceMeasurer extends PerformanceMeasurer {
private performanceObserver?: PerformanceObserver

constructor() {
// eslint-disable-next-line @typescript-eslint/no-var-requires
super(PerformanceFactory.getPerformance())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function keywordsWithMockCms(
stemBlackList: { [locale: string]: StemmingBlackList[] } = {}
): SearchByKeywords {
const mockCms = mock(DummyCMS)
when(mockCms.contentsWithKeywords(deepEqual(context))).thenResolve(
when(mockCms.contentsWithKeywords(deepEqual(context), undefined)).thenResolve(
asastre marked this conversation as resolved.
Show resolved Hide resolved
allContents
)
const normalizer = new Normalizer(stemBlackList)
Expand Down