From a35d203c81c8acca763ee3cddc668f55efd8b88a Mon Sep 17 00:00:00 2001 From: Jan Melcher Date: Wed, 11 Sep 2024 15:15:06 +0200 Subject: [PATCH] feat: consider indices in withModuleSelection() --- .../project/select-modules-in-sources.spec.ts | 226 ++++++++++++++++++ src/project/select-modules-in-sources.ts | 161 ++++++++++++- 2 files changed, 379 insertions(+), 8 deletions(-) diff --git a/spec/project/select-modules-in-sources.spec.ts b/spec/project/select-modules-in-sources.spec.ts index e9043643..05d4eccf 100644 --- a/spec/project/select-modules-in-sources.spec.ts +++ b/spec/project/select-modules-in-sources.spec.ts @@ -117,6 +117,232 @@ describe('selectModulesInProjectSource', () => { } `); }); + + it('keeps indices that are fully covered by the modules', () => { + const result = run( + gql` + type Test + @rootEntity( + indices: [ + { fields: ["field1", "key"] } + { fields: ["field1", "children.field1"], sparse: true } + ] + ) + @modules(in: "module1") { + key: String @key @modules(in: "module1") + field1: String @modules(in: "module1") + children: [Child] @modules(in: "module1") + } + + type Child @childEntity @modules(in: "module1") { + field1: String @modules(in: "module1") + field2: String @modules(in: "module1") + } + `, + ['module1'], + ); + expect(result).to.equal(` + type Test + @rootEntity( + indices: [ + { fields: ["field1", "key"] } + { fields: ["field1", "children.field1"], sparse: true } + ] + ) + @modules(in: ["module1"]) { + key: String @key @modules(in: ["module1"]) + field1: String @modules(in: ["module1"]) + children: [Child] @modules(in: ["module1"]) + } + + type Child @childEntity @modules(in: ["module1"]) { + field1: String @modules(in: ["module1"]) + field2: String @modules(in: ["module1"]) + } + `); + }); + + it('removes indices where none of the specified fields are included in the selected modules', () => { + const result = run( + gql` + type Test + @rootEntity( + indices: [ + { fields: ["field1", "key"] } + { fields: ["field2", "children.field2"], sparse: true } + ] + ) + @modules(in: "module1") { + key: String @key @modules(in: "module1") + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + children: [Child] @modules(in: "module1") + } + + type Child @childEntity @modules(in: "module1") { + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + } + `, + ['module1'], + ); + expect(result).to.equal(` + type Test + @rootEntity( + indices: [ + { fields: ["field1", "key"] } + + ] + ) + @modules(in: ["module1"]) { + key: String @key @modules(in: ["module1"]) + field1: String @modules(in: ["module1"]) + + children: [Child] @modules(in: ["module1"]) + } + + type Child @childEntity @modules(in: ["module1"]) { + field1: String @modules(in: ["module1"]) + + } + `); + }); + + it('removes the whole indices arg if all indices have been removed', () => { + const result = run( + gql` + type Test + @rootEntity( + indices: [ + { fields: ["field2"] } + { fields: ["field2", "children.field2"], sparse: true } + ] + ) + @modules(in: "module1") { + key: String @key @modules(in: "module1") + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + children: [Child] @modules(in: "module1") + } + + type Child @childEntity @modules(in: "module1") { + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + } + `, + ['module1'], + ); + expect(result).to.equal(` + type Test + @rootEntity + @modules(in: ["module1"]) { + key: String @key @modules(in: ["module1"]) + field1: String @modules(in: ["module1"]) + + children: [Child] @modules(in: ["module1"]) + } + + type Child @childEntity @modules(in: ["module1"]) { + field1: String @modules(in: ["module1"]) + + } + `); + }); + + it('removes the whole indices arg if all indices have been removed (and there are other args)', () => { + const result = run( + gql` + type Test + @rootEntity( + flexSearch: true + indices: [ + { fields: ["field2"] } + { fields: ["field2", "children.field2"], sparse: true } + ] + ) + @modules(in: "module1") { + key: String @key @modules(in: "module1") + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + children: [Child] @modules(in: "module1") + } + + type Child @childEntity @modules(in: "module1") { + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + } + `, + ['module1'], + ); + expect(result).to.equal(` + type Test + @rootEntity( + flexSearch: true + + ) + @modules(in: ["module1"]) { + key: String @key @modules(in: ["module1"]) + field1: String @modules(in: ["module1"]) + + children: [Child] @modules(in: ["module1"]) + } + + type Child @childEntity @modules(in: ["module1"]) { + field1: String @modules(in: ["module1"]) + + } + `); + }); + + it('trims down indices that are partially covered by the modules', () => { + const result = run( + gql` + type Test + @rootEntity( + indices: [ + { fields: ["field1", "key"] } + { + fields: ["field1", "children.field1", "children.field2"] + sparse: true + } + ] + ) + @modules(in: "module1") { + key: String @key @modules(in: "module1") + field1: String @modules(in: "module1") + children: [Child] @modules(in: "module1") + } + + type Child @childEntity @modules(in: "module1") { + field1: String @modules(in: "module1") + field2: String @modules(in: "module2") + } + `, + ['module1'], + ); + expect(result).to.equal(` + type Test + @rootEntity( + indices: [ + { fields: ["field1", "key"] } + { + fields: ["field1", "children.field1"] + sparse: true + } + ] + ) + @modules(in: ["module1"]) { + key: String @key @modules(in: ["module1"]) + field1: String @modules(in: ["module1"]) + children: [Child] @modules(in: ["module1"]) + } + + type Child @childEntity @modules(in: ["module1"]) { + field1: String @modules(in: ["module1"]) + + } + `); + }); }); describe('with removeModuleDeclarations = true', () => { diff --git a/src/project/select-modules-in-sources.ts b/src/project/select-modules-in-sources.ts index e09ac0d3..51d5beb2 100644 --- a/src/project/select-modules-in-sources.ts +++ b/src/project/select-modules-in-sources.ts @@ -1,21 +1,29 @@ import { DirectiveNode, FieldDefinitionNode, + isTypeDefinitionNode, Kind, + ListValueNode, Location, - TypeDefinitionNode, - isTypeDefinitionNode, + ObjectValueNode, print, + StringValueNode, + TypeDefinitionNode, } from 'graphql'; import { ParsedGraphQLProjectSource, ParsedObjectProjectSource, ParsedProjectSourceBaseKind, } from '../config/parsed-project'; -import { Model, ValidationMessage } from '../model'; +import { IndexField, Model, RootEntityType, ValidationMessage } from '../model'; import { parseModuleSpecificationExpression } from '../model/implementation/modules/expression-parser'; import { ValidationContext } from '../model/validation/validation-context'; -import { MODULES_DIRECTIVE, MODULES_IN_ARG } from '../schema/constants'; +import { + INDICES_ARG, + MODULES_DIRECTIVE, + MODULES_IN_ARG, + ROOT_ENTITY_DIRECTIVE, +} from '../schema/constants'; import { parseProjectSource } from '../schema/schema-builder'; import { findDirectiveWithName } from '../schema/schema-utils'; import { Project, ProjectOptions } from './project'; @@ -218,6 +226,10 @@ function selectModulesInGraphQLSource({ continue; } + if (type.isRootEntityType) { + changes.push(...changeIndicesIfNecessary(typeDef, type, selectedModules)); + } + if (!typeDef.fields) { continue; } @@ -262,13 +274,14 @@ function selectModulesInGraphQLSource({ let currentPosition = 0; let output = ''; - for (let i = 0; i <= changes.length; i++) { + const sortedChanges = [...changes].sort((a, b) => a.location.start - b.location.start); + for (let i = 0; i <= sortedChanges.length; i++) { // TODO expand the spans to include leading and trailing trivia (such as comments and whitespace) - const change = changes[i] as Change | undefined; + const change = sortedChanges[i] as Change | undefined; const includeUntilIndex = change ? change.location.start : source.body.length; if (includeUntilIndex < currentPosition) { throw new Error( - `Changes are not in order: found ${includeUntilIndex} after ${currentPosition}`, + `Changes are overlapping: found ${includeUntilIndex} after ${currentPosition}`, ); } output += source.body.substring(currentPosition, includeUntilIndex); @@ -276,7 +289,7 @@ function selectModulesInGraphQLSource({ output += change.replacement; } if (change) { - currentPosition = changes[i].location.end; + currentPosition = change.location.end; } } @@ -359,3 +372,135 @@ function changeModuleDirectiveIfNecessary( const replacement = print(newDirective); return { location: modulesDirectve.loc, replacement }; } + +function changeIndicesIfNecessary( + definitionNode: TypeDefinitionNode, + type: RootEntityType, + selectedModules: ReadonlySet, +): ReadonlyArray { + const rootEntityDirective = findDirectiveWithName(definitionNode, ROOT_ENTITY_DIRECTIVE); + if (!rootEntityDirective || !rootEntityDirective.loc) { + return []; + } + const indicesArg = rootEntityDirective.arguments?.find((a) => a.name.value === INDICES_ARG); + if (!indicesArg || !indicesArg.loc || indicesArg.value.kind !== Kind.LIST) { + // nothing to do when there is no indices arg or if it has an invalid value (not a list), + // and nothing we can do if it does not have a location + return []; + } + + const indexDefs = indicesArg.value.values.filter((v) => v.kind === Kind.OBJECT); + const changes: Change[] = []; + let removedIndexCount = 0; + for (const indexDef of indexDefs) { + if (!indexDef.loc) { + // if we don't have a location, cannot remove or change anything + continue; + } + const change = changeIndexIfNecessary(indexDef, type, selectedModules); + if (change.remove) { + removedIndexCount++; + changes.push({ location: indexDef.loc }); + } else if (change.changeFields) { + const fieldsArg = indexDef.fields.find((f) => f.name.value === 'fields'); + if (fieldsArg && fieldsArg.value.loc) { + const valueNode: ListValueNode = { + kind: Kind.LIST, + values: change.changeFields.map( + (field): StringValueNode => ({ + kind: Kind.STRING, + value: field, + }), + ), + }; + changes.push({ location: fieldsArg.value.loc, replacement: print(valueNode) }); + } + } + } + + // If we're removing all indices, rather remove the whole argument + if (removedIndexCount === indicesArg.value.values.length) { + if (rootEntityDirective.arguments?.length === 1) { + // if there was no other argument besides "indices", just re-emit the @rootEntity + // directive (we need to remove the (), and there is nothing else that would get + // re-formatted by this operation + const newDirective: DirectiveNode = { + ...rootEntityDirective, + arguments: undefined, + }; + return [{ location: rootEntityDirective.loc, replacement: print(newDirective) }]; + } else { + return [{ location: indicesArg.loc }]; + } + } + + // does not delete potential commas between the indices. However, indices are usually so long + // that there is one per line, and you probably don't use commas then (prettier does not) + // it does leave a lot of whitespace, but that's a common problem (and can be fixed by running + // prettier after withModuleSelection()) + return changes; +} + +interface IndexChange { + readonly remove?: boolean; + readonly changeFields?: ReadonlyArray; +} + +function changeIndexIfNecessary( + indexDef: ObjectValueNode, + type: RootEntityType, + selectedModules: ReadonlySet, +): IndexChange { + const fieldsArg = indexDef.fields.find((f) => f.name.value === 'fields'); + if (!fieldsArg) { + // invalid index - do not change anything + return {}; + } + let fields: ReadonlyArray; + if (fieldsArg.value.kind === Kind.STRING) { + fields = [fieldsArg.value.value]; + } else if (fieldsArg.value.kind === Kind.LIST) { + fields = fieldsArg.value.values.filter((v) => v.kind === Kind.STRING).map((v) => v.value); + } else { + // invalid index - do not change anything + return {}; + } + + const fieldsSoFar: string[] = []; + for (const field of fields) { + // it's easier to reconstruct the IndexField instances than using type.indices because + // we would need to find the matching IndexField instances + const indexField = new IndexField(field, type, undefined); + const fieldsInPath = indexField.fieldsInPath; + if (!fieldsInPath) { + // if there is an error resolving the field paths, we can't really do anything + // behave like in other situations here and in doubt do not touch the definition + return {}; + } + + if (fieldsInPath.some((f) => !f.effectiveModuleSpecification.includedIn(selectedModules))) { + // The index is not fully covered by the selected modules + const uniqueArg = indexDef.fields.find((f) => f.name.value === 'unique'); + if (uniqueArg && uniqueArg.value.kind === Kind.BOOLEAN && uniqueArg.value.value) { + // A unique index that is not fully covered needs to be removed + // (if we were to keep a prefix of the index, the unique behavior would change) + return { remove: true }; + } + + if (fieldsSoFar.length) { + // for non-unique indices, include a prefix index + // TODO only include this prefix index if there isn't already an identical one + // (but first discuss whether the prefix-trimming behavior is actually what we want + // - it can be confusing because it only applies to non-unique indices) + return { changeFields: fieldsSoFar }; + } else { + return { remove: true }; + } + } + + fieldsSoFar.push(field); + } + + // all fields are covered - no change needed + return {}; +}