diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44de9baf9..007366a9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,7 @@ jobs: strategy: matrix: arango-image: ['arangodb:3.6', 'arangodb:3.7', 'arangodb:3.8'] - node-version: [12.x, 14.x, 16.x] + node-version: [14.x, 16.x] steps: - uses: actions/checkout@v2 - name: Use Node.js ${{ matrix.node-version }} diff --git a/core-exports.ts b/core-exports.ts index 3b226d80e..112176578 100644 --- a/core-exports.ts +++ b/core-exports.ts @@ -1,4 +1,4 @@ -export { RequestProfile, ProjectOptions, RequestContext, ModelValidationOptions } from './src/config/interfaces'; +export { RequestProfile, ProjectOptions, RequestContext, ModelOptions } from './src/config/interfaces'; export { FieldResolverParameters } from './src/graphql/operation-based-resolvers'; export { Project, ProjectConfig } from './src/project/project'; export { InvalidProjectError } from './src/project/invalid-project-error'; diff --git a/spec/regression/papers/model/paper.graphqls b/spec/regression/papers/model/paper.graphqls index 1c1474c10..33ab63f2e 100644 --- a/spec/regression/papers/model/paper.graphqls +++ b/spec/regression/papers/model/paper.graphqls @@ -6,8 +6,11 @@ enum Category { Programming } +# need to specify flexSearchOrder with an id field so createdAt does not included (which we cannot test because it changes) "A scientific paper" -type Paper @rootEntity(flexSearch: true, flexSearchLanguage: EN) @roles(readWrite: ["admin"]) { +type Paper + @rootEntity(flexSearch: true, flexSearchLanguage: EN, flexSearchOrder: [{ field: "id", direction: ASC }]) + @roles(readWrite: ["admin"]) { key: String @key @flexSearch # need a key field for the reference title: String @index @flexSearch # for pagination performance test "The date this paper has been published in a scientific journal or conference" diff --git a/spec/regression/regression-suite.ts b/spec/regression/regression-suite.ts index 42e635b8b..c92e5f774 100644 --- a/spec/regression/regression-suite.ts +++ b/spec/regression/regression-suite.ts @@ -71,7 +71,7 @@ export class RegressionSuite { authRoles: context.authRoles, flexSearchMaxFilterableAndSortableAmount: context.flexSearchMaxFilterableAndSortableAmount }), - modelValidationOptions: { + modelOptions: { forbiddenRootEntityNames: [] }, ...options, diff --git a/src/config/interfaces.ts b/src/config/interfaces.ts index 17cc435f5..7ac4e83bf 100644 --- a/src/config/interfaces.ts +++ b/src/config/interfaces.ts @@ -25,13 +25,6 @@ export interface SchemaOptions { readonly maxOrderByRootEntityDepth?: number; } -export interface ModelValidationOptions { - /** - * A list of root entity names that are not allowed. - */ - readonly forbiddenRootEntityNames?: ReadonlyArray; -} - export interface ModelOptions { /** * Determines whether a slash in a source name indicates the target namespace for that source @@ -39,6 +32,18 @@ export interface ModelOptions { * Defaults to true. Explicitly specify false to disable this. */ readonly useSourceDirectoriesAsNamespaces?: boolean; + + /** + * Specifies the default for case-sensitiveness of flexSearch fields (can be overridden with the decorator) + * + * Default is false + */ + readonly isFlexSearchIndexCaseSensitiveByDefault?: boolean; + + /** + * A list of root entity names that are not allowed. + */ + readonly forbiddenRootEntityNames?: ReadonlyArray; } export interface ProjectOptions { @@ -47,7 +52,7 @@ export interface ProjectOptions { readonly getExecutionOptions?: (args: ExecutionOptionsCallbackArgs) => ExecutionOptions; readonly schemaOptions?: SchemaOptions; - readonly modelValidationOptions?: ModelValidationOptions; + readonly modelOptions?: ModelOptions; /** diff --git a/src/database/inmemory/js-generator.ts b/src/database/inmemory/js-generator.ts index c298f5dec..e6128e192 100644 --- a/src/database/inmemory/js-generator.ts +++ b/src/database/inmemory/js-generator.ts @@ -964,11 +964,11 @@ register(OperatorWithAnalyzerQueryNode, (node, context) => { let rhs = processNode(node.rhs, context); if (isCaseInsensitive) { - lhs = js`${lhs}.toLowerCase()`; + lhs = js`(${lhs})?.toLowerCase()`; const rhsVar = js.variable('rhs'); rhs = jsExt.evaluatingLambda( rhsVar, - js`(Array.isArray(${rhsVar}) ? ${rhsVar}.map(value => value.toLowerCase()) : ${rhsVar}.toLowerCase())`, + js`(Array.isArray(${rhsVar}) ? ${rhsVar}.map(value => value?.toLowerCase()) : (${rhsVar})?.toLowerCase())`, rhs ); } diff --git a/src/model/config/indices.ts b/src/model/config/indices.ts index 629febda6..4180b540d 100644 --- a/src/model/config/indices.ts +++ b/src/model/config/indices.ts @@ -1,4 +1,5 @@ import { DirectiveNode, ObjectValueNode, StringValueNode } from 'graphql'; +import { OrderDirection } from '../implementation/order'; export interface IndexDefinitionConfig { readonly name?: string; @@ -23,7 +24,7 @@ export interface IndexDefinitionConfig { export interface FlexSearchPrimarySortClauseConfig { readonly field: string; - readonly asc: boolean; + readonly direction: OrderDirection; } export interface FlexSearchIndexConfig { diff --git a/src/model/config/model.ts b/src/model/config/model.ts index e03477a3b..6e52172f9 100644 --- a/src/model/config/model.ts +++ b/src/model/config/model.ts @@ -1,4 +1,4 @@ -import { ModelValidationOptions } from '../../config/interfaces'; +import { ModelOptions } from '../../config/interfaces'; import { ValidationMessage } from '../validation'; import { BillingConfig } from './billing'; import { LocalizationConfig } from './i18n'; @@ -11,6 +11,6 @@ export interface ModelConfig { readonly validationMessages?: ReadonlyArray; readonly i18n?: ReadonlyArray; readonly billing?: BillingConfig; - readonly modelValidationOptions?: ModelValidationOptions; readonly timeToLiveConfigs?: ReadonlyArray; + readonly options?: ModelOptions; } diff --git a/src/model/create-model.ts b/src/model/create-model.ts index 49178bd92..5521718ae 100644 --- a/src/model/create-model.ts +++ b/src/model/create-model.ts @@ -16,7 +16,7 @@ import { TypeDefinitionNode, valueFromAST } from 'graphql'; -import { ModelValidationOptions } from '../config/interfaces'; +import { ModelOptions } from '../config/interfaces'; import { ParsedGraphQLProjectSource, ParsedObjectProjectSource, @@ -110,21 +110,22 @@ import { } from './config'; import { BillingConfig } from './config/billing'; import { Model } from './implementation'; +import { OrderDirection } from './implementation/order'; import { parseBillingConfigs } from './parse-billing'; import { parseI18nConfigs } from './parse-i18n'; import { parseTTLConfigs } from './parse-ttl'; import { ValidationContext, ValidationMessage } from './validation'; -export function createModel(parsedProject: ParsedProject, modelValidationOptions?: ModelValidationOptions): Model { +export function createModel(parsedProject: ParsedProject, options?: ModelOptions): Model { const validationContext = new ValidationContext(); return new Model({ - types: createTypeInputs(parsedProject, validationContext), + types: createTypeInputs(parsedProject, validationContext, options ?? {}), permissionProfiles: extractPermissionProfiles(parsedProject), i18n: extractI18n(parsedProject), validationMessages: validationContext.validationMessages, billing: extractBilling(parsedProject), - modelValidationOptions, - timeToLiveConfigs: extractTimeToLive(parsedProject) + timeToLiveConfigs: extractTimeToLive(parsedProject), + options }); } @@ -144,7 +145,11 @@ const VALIDATION_ERROR_MISSING_OBJECT_TYPE_DIRECTIVE = `Add one of @${ROOT_ENTIT const VALIDATION_ERROR_INVALID_DEFINITION_KIND = 'This kind of definition is not allowed. Only object and enum type definitions are allowed.'; -function createTypeInputs(parsedProject: ParsedProject, context: ValidationContext): ReadonlyArray { +function createTypeInputs( + parsedProject: ParsedProject, + context: ValidationContext, + options: ModelOptions +): ReadonlyArray { const graphQLSchemaParts = parsedProject.sources.filter( parsedSource => parsedSource.kind === ParsedProjectSourceBaseKind.GRAPHQL ) as ReadonlyArray; @@ -173,7 +178,7 @@ function createTypeInputs(parsedProject: ParsedProject, context: ValidationConte }; return enumTypeInput; case OBJECT_TYPE_DEFINITION: - return createObjectTypeInput(definition, schemaPart, context); + return createObjectTypeInput(definition, schemaPart, context, options); default: return undefined; } @@ -196,7 +201,8 @@ function createEnumValues(valueNodes: ReadonlyArray): R function createObjectTypeInput( definition: ObjectTypeDefinitionNode, schemaPart: ParsedGraphQLProjectSource, - context: ValidationContext + context: ValidationContext, + options: ModelOptions ): ObjectTypeConfig { const entityType = getKindOfObjectTypeNode(definition, context); @@ -204,7 +210,7 @@ function createObjectTypeInput( name: definition.name.value, description: definition.description ? definition.description.value : undefined, astNode: definition, - fields: (definition.fields || []).map(field => createFieldInput(field, context)), + fields: (definition.fields || []).map(field => createFieldInput(field, context, options)), namespacePath: getNamespacePath(definition, schemaPart.namespacePath), flexSearchLanguage: getDefaultLanguage(definition, context) }; @@ -337,7 +343,7 @@ function getFlexSearchOrder(rootEntityDirective?: DirectiveNode): ReadonlyArray< .map((value: any) => { return { field: value.field, - asc: value.direction === 'ASC' ? true : false + direction: value.direction === 'DESC' ? OrderDirection.DESCENDING : OrderDirection.ASCENDING }; }); } @@ -457,7 +463,11 @@ function getLanguage(fieldNode: FieldDefinitionNode, context: ValidationContext) } } -function createFieldInput(fieldNode: FieldDefinitionNode, context: ValidationContext): FieldConfig { +function createFieldInput( + fieldNode: FieldDefinitionNode, + context: ValidationContext, + options: ModelOptions +): FieldConfig { const inverseOfASTNode = getInverseOfASTNode(fieldNode, context); const relationDeleteActionASTNode = getRelationDeleteActionASTNode(fieldNode, context); const referenceDirectiveASTNode = findDirectiveWithName(fieldNode, REFERENCE_DIRECTIVE); @@ -500,7 +510,7 @@ function createFieldInput(fieldNode: FieldDefinitionNode, context: ValidationCon isFlexSearchIndexCaseSensitive: flexSearchIndexCaseSensitiveNode?.value.kind === 'BooleanValue' ? flexSearchIndexCaseSensitiveNode.value.value - : undefined, + : options.isFlexSearchIndexCaseSensitiveByDefault, isFlexSearchIndexedASTNode: findDirectiveWithName(fieldNode, FLEX_SEARCH_INDEXED_DIRECTIVE), isFlexSearchFulltextIndexed: hasDirectiveWithName(fieldNode, FLEX_SEARCH_FULLTEXT_INDEXED_DIRECTIVE), isFlexSearchFulltextIndexedASTNode: findDirectiveWithName(fieldNode, FLEX_SEARCH_FULLTEXT_INDEXED_DIRECTIVE), diff --git a/src/model/implementation/field.ts b/src/model/implementation/field.ts index 5e245049a..837bc0f8b 100644 --- a/src/model/implementation/field.ts +++ b/src/model/implementation/field.ts @@ -1522,7 +1522,7 @@ export class Field implements ModelComponent { } get isFlexSearchIndexCaseSensitive(): boolean { - return this.input.isFlexSearchIndexCaseSensitive ?? true; + return this.input.isFlexSearchIndexCaseSensitive ?? false; } get flexSearchAnalyzer(): string | undefined { diff --git a/src/model/implementation/model.ts b/src/model/implementation/model.ts index 5bb46fdc7..2a27d48d6 100644 --- a/src/model/implementation/model.ts +++ b/src/model/implementation/model.ts @@ -1,6 +1,6 @@ import { groupBy, uniqBy } from 'lodash'; import memorize from 'memorize-decorator'; -import { ModelValidationOptions } from '../../config/interfaces'; +import { ModelOptions } from '../../config/interfaces'; import { flatMap, objectEntries, objectValues } from '../../utils/utils'; import { ModelConfig, TypeKind } from '../config'; import { NamespacedPermissionProfileConfigMap } from '../index'; @@ -31,7 +31,11 @@ export class Model implements ModelComponent { readonly i18n: ModelI18n; readonly permissionProfiles: ReadonlyArray; readonly billingEntityTypes: ReadonlyArray; - readonly modelValidationOptions?: ModelValidationOptions; + /** + * @deprecated use options + */ + readonly modelValidationOptions?: ModelOptions; + readonly options?: ModelOptions; readonly timeToLiveTypes: ReadonlyArray; constructor(private input: ModelConfig) { @@ -50,7 +54,8 @@ export class Model implements ModelComponent { this.billingEntityTypes = input.billing ? input.billing.billingEntities.map(value => new BillingEntityType(value, this)) : []; - this.modelValidationOptions = input.modelValidationOptions; + this.options = input.options; + this.modelValidationOptions = input.options; this.timeToLiveTypes = input.timeToLiveConfigs ? input.timeToLiveConfigs.map(ttlConfig => new TimeToLiveType(ttlConfig, this)) : []; @@ -243,10 +248,10 @@ export class Model implements ModelComponent { } get forbiddenRootEntityNames(): ReadonlyArray { - if (!this.modelValidationOptions || !this.modelValidationOptions.forbiddenRootEntityNames) { + if (!this.options || !this.options.forbiddenRootEntityNames) { return ['BillingEntity']; } - return this.modelValidationOptions!.forbiddenRootEntityNames; + return this.options!.forbiddenRootEntityNames; } } diff --git a/src/model/implementation/root-entity-type.ts b/src/model/implementation/root-entity-type.ts index c5f923777..9d9fda5b1 100644 --- a/src/model/implementation/root-entity-type.ts +++ b/src/model/implementation/root-entity-type.ts @@ -2,6 +2,7 @@ import { GraphQLID, GraphQLString } from 'graphql'; import memorize from 'memorize-decorator'; import { ACCESS_GROUP_FIELD, + ENTITY_CREATED_AT, FLEX_SEARCH_FULLTEXT_INDEXED_DIRECTIVE, FLEX_SEARCH_INDEXED_DIRECTIVE, FLEX_SEARCH_ORDER_ARGUMENT, @@ -349,21 +350,29 @@ export class RootEntityType extends ObjectTypeBase { // primary sort is only used for sorting, so make sure it's unique // - this makes querying more consistent // - this enables us to use primary sort for cursor-based pagination (which requires an absolute sort order) + // the default primary sort should be createdAt_DESC, because this is useful most of the time. to avoid + // surprises when you do specify a primary sort, always add this default at the end (as long as it's not already + // included in the index) + if (!clauses.some(clause => clause.field === ENTITY_CREATED_AT)) { + clauses = [ + ...clauses, + { + field: ENTITY_CREATED_AT, + direction: OrderDirection.DESCENDING + } + ]; + } if (!clauses.some(clause => clause.field === this.discriminatorField.name)) { clauses = [ ...clauses, { field: this.discriminatorField.name, - asc: true + direction: OrderDirection.DESCENDING } ]; } return clauses.map( - c => - new FlexSearchPrimarySortClause( - new FieldPath({ path: c.field, baseType: this }), - c.asc ? OrderDirection.ASCENDING : OrderDirection.DESCENDING - ) + c => new FlexSearchPrimarySortClause(new FieldPath({ path: c.field, baseType: this }), c.direction) ); } diff --git a/src/project/project.ts b/src/project/project.ts index 4c25a6c41..da998d9b3 100644 --- a/src/project/project.ts +++ b/src/project/project.ts @@ -118,7 +118,6 @@ export class Project { getOperationIdentifier: config.getOperationIdentifier, processError: config.processError, schemaOptions: config.schemaOptions, - modelValidationOptions: config.modelValidationOptions, modelOptions: config.modelOptions }; } diff --git a/src/schema-generation/flex-search-filter-input-types/filter-fields.ts b/src/schema-generation/flex-search-filter-input-types/filter-fields.ts index ae17bb1fc..6cb0d42ec 100644 --- a/src/schema-generation/flex-search-filter-input-types/filter-fields.ts +++ b/src/schema-generation/flex-search-filter-input-types/filter-fields.ts @@ -389,6 +389,22 @@ export function resolveFilterField( not(new FlexSearchFieldExistsQueryNode(valueNode, analyzer)) ); } + // field < x and field <= x should also find NULL values, because that's how it behaves in non-flexsearch case + if ( + (filterField.operatorName === INPUT_FIELD_LT || filterField.operatorName === INPUT_FIELD_LTE) && + filterValue != null + ) { + const isNull = new BinaryOperationQueryNode( + new BinaryOperationQueryNode(valueNode, BinaryOperator.EQUAL, NullQueryNode.NULL), + BinaryOperator.OR, + not(new FlexSearchFieldExistsQueryNode(valueNode, analyzer)) + ); + return new BinaryOperationQueryNode( + isNull, + BinaryOperator.OR, + filterField.resolveOperator(valueNode, literalNode, analyzer) + ); + } if (filterField.operatorName == INPUT_FIELD_IN && Array.isArray(filterValue) && filterValue.includes(null)) { return new BinaryOperationQueryNode( filterField.resolveOperator(valueNode, literalNode, analyzer), diff --git a/src/schema-generation/order-by-and-pagination-augmentation.ts b/src/schema-generation/order-by-and-pagination-augmentation.ts index e07d3b837..df79a740f 100644 --- a/src/schema-generation/order-by-and-pagination-augmentation.ts +++ b/src/schema-generation/order-by-and-pagination-augmentation.ts @@ -119,33 +119,24 @@ export class OrderByAndPaginationAugmentation { // flexsearch? // we only offer cursor-based pagination if we can cover all required filters with flex search filters // this means we just replace the flexsearch listNode with a flexsearch listNode that does the cursor-filtering - let leaveUnordered = false; if (listNode instanceof FlexSearchQueryNode) { if (!orderByType) { throw new Error(`OrderBy type missing for flex search`); } - // whenever possible, use primary sort - if (orderArgMatchesPrimarySort(args[ORDER_BY_ARG], listNode.rootEntityType.flexSearchPrimarySort)) { - // this would be cleaner if the primary sort was actually parsed into a ModelComponent (see e.g. the Index and IndexField classes) - orderByValues = listNode.rootEntityType.flexSearchPrimarySort.map(clause => - orderByType.getValueOrThrow( - clause.field.path.replace('.', '_') + - (clause.direction === OrderDirection.ASCENDING ? '_ASC' : '_DESC') - ) - ); - leaveUnordered = true; - } else { - // this will generate a SORT clause that's not covered by the flexsearch index, - // but the TooManyObject check of flex-search-generator already handles this case to throw - // a TooManyObjects error if needed. - orderByValues = getOrderByValues(args, orderByType, { isAbsoluteOrderRequired }); - } + // we used to remove the sort clause if it matched the primary sort, but it turned out this is not + // ok. If the sorting matches the primary sort, sorting is efficient, but you still need to specify + // it, otherwise, sometimes the order can be wrong (after inserting or updating documents). + + // this may generate a SORT clause that is not covered by the flexsearch index, + // but the TooManyObject check of flex-search-generator already handles this case to throw + // a TooManyObjects error if needed. + orderByValues = getOrderByValues(args, orderByType, { isAbsoluteOrderRequired }); // for now, cursor-based pagination is only allowed when we can use flexsearch filters for the // paginationFilter. This simplifies the implementation, but maybe we should support it in the // future for consistency. Then however we need to adjust the too-many-objects check // do this check also if cursor is just requested so we get this error on the first page - if (isCursorRequested || !!afterArg) { + if (isCursorRequested || afterArg) { const violatingClauses = orderByValues.filter(val => val.path.some(field => !field.isFlexSearchIndexed) ); @@ -189,10 +180,9 @@ export class OrderByAndPaginationAugmentation { } // sorting always happens on the TransformListQueryNode and not in the FlexSearchQueryNode - const orderBy = - !orderByType || leaveUnordered - ? OrderSpecification.UNORDERED - : new OrderSpecification(orderByValues.map(value => value.getClause(objectNode))); + const orderBy = !orderByType + ? OrderSpecification.UNORDERED + : new OrderSpecification(orderByValues.map(value => value.getClause(objectNode))); if (orderBy.isUnordered() && maxCount == undefined && paginationFilter === ConstBoolQueryNode.TRUE) { return originalListNode; diff --git a/src/schema-generation/utils/flex-search-utils.ts b/src/schema-generation/utils/flex-search-utils.ts index 8e4c70768..7c30845d1 100644 --- a/src/schema-generation/utils/flex-search-utils.ts +++ b/src/schema-generation/utils/flex-search-utils.ts @@ -5,6 +5,7 @@ export function orderArgMatchesPrimarySort( clauses: ReadonlyArray | undefined, primarySort: ReadonlyArray ): boolean { + // TODO what about sort clauses that are added automatically because the user used cursor-based pagination? if (!clauses || !clauses.length) { return true; } diff --git a/src/schema/schema-builder.ts b/src/schema/schema-builder.ts index e3bea1af8..4fefa8993 100644 --- a/src/schema/schema-builder.ts +++ b/src/schema/schema-builder.ts @@ -81,7 +81,7 @@ export function validateAndPrepareSchema(project: Project): { validationResult: const preparedProject = executePreMergeTransformationPipeline({ sources: validParsedSources }); - const model = createModel(preparedProject, project.options.modelValidationOptions); + const model = createModel(preparedProject, project.options.modelOptions); const mergedSchema: DocumentNode = mergeSchemaDefinition(preparedProject);