From 54335a9b03f70cd7837bf3275bc2700d3a9ad502 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Mon, 19 Feb 2024 16:34:16 +0100 Subject: [PATCH 1/9] Add composable indent --- lib/file.js | 12 +++++++++++ lib/visitor.js | 54 +++++++++++++++++++++++++------------------------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/lib/file.js b/lib/file.js index dc785da8..cae5849c 100644 --- a/lib/file.js +++ b/lib/file.js @@ -480,6 +480,18 @@ class Buffer { add(part) { this.parts.push(this.currentIndent + part) } + + /** + * Adds an element to the buffer with one level of indent. + * @param {string | (() => string)} part either a string or a function that returns a string. + */ + addIndented(part) { + this.indent() + this.add(typeof part === 'function' + ? part() + : part) + this.outdent() + } } /** diff --git a/lib/visitor.js b/lib/visitor.js index e2d7589b..c65bda2d 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -206,25 +206,25 @@ class Visitor { } } - buffer.indent() - for (const e of enums) { - buffer.add(`static ${e.name} = ${propertyToInlineEnumName(clean, e.name)}`) - file.addInlineEnum(clean, name, e.name, csnToEnumPairs(e, {unwrapVals: true})) - } - buffer.add('static actions: {') - buffer.indent() - for (const [aname, action] of Object.entries(entity.actions ?? {})) { - buffer.add( - SourceFile.stringifyLambda({ - name: aname, - parameters: this.#stringifyFunctionParams(action.params, file), - returns: action.returns ? this.resolver.resolveAndRequire(action.returns, file).typeName : 'any' - //initialiser: `undefined as unknown as typeof ${clean}.${aname}`, - }) - ) - } - buffer.outdent() - buffer.outdent() + buffer.addIndented(() => { + for (const e of enums) { + buffer.add(`static ${e.name} = ${propertyToInlineEnumName(clean, e.name)}`) + file.addInlineEnum(clean, name, e.name, csnToEnumPairs(e, {unwrapVals: true})) + } + buffer.add('static actions: {') + buffer.addIndented(function() { + for (const [aname, action] of Object.entries(entity.actions ?? {})) { + buffer.add( + SourceFile.stringifyLambda({ + name: aname, + parameters: this.#stringifyFunctionParams(action.params, file), + returns: action.returns ? this.resolver.resolveAndRequire(action.returns, file).typeName : 'any' + //initialiser: `undefined as unknown as typeof ${clean}.${aname}`, + }) + ) + } + }.bind(this)) + }) buffer.add('}') // end of actions buffer.outdent() @@ -409,14 +409,14 @@ class Visitor { const buffer = file.events.buffer buffer.add('// event') buffer.add(`export class ${clean} {`) - buffer.indent() - const propOpt = this.options.propertiesOptional - this.options.propertiesOptional = false - for (const [ename, element] of Object.entries(event.elements ?? {})) { - this.visitElement(ename, element, file, buffer) - } - this.options.propertiesOptional = propOpt - buffer.outdent() + buffer.addIndented(() => { + const propOpt = this.options.propertiesOptional + this.options.propertiesOptional = false + for (const [ename, element] of Object.entries(event.elements ?? {})) { + this.visitElement(ename, element, file, buffer) + } + this.options.propertiesOptional = propOpt + }) buffer.add('}') } From f2983582baf89b4f9fd5fc69ff0d820f418e34d9 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Tue, 20 Feb 2024 18:33:58 +0100 Subject: [PATCH 2/9] Improve addIndented --- lib/file.js | 11 +++++++---- lib/visitor.js | 29 +++++++++++------------------ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/lib/file.js b/lib/file.js index cae5849c..b6dd2e69 100644 --- a/lib/file.js +++ b/lib/file.js @@ -483,13 +483,16 @@ class Buffer { /** * Adds an element to the buffer with one level of indent. - * @param {string | (() => string)} part either a string or a function that returns a string. + * @param {string | (() => void)} part either a string or a function. If it is a string, it is added to the buffer. + * If not, it is expected to be a function that manipulates the buffer as a side effect. */ addIndented(part) { this.indent() - this.add(typeof part === 'function' - ? part() - : part) + if (typeof part === 'function') { + part() + } else if (Array.isArray(part)) { + part.forEach(p => this.add(p)) + } this.outdent() } } diff --git a/lib/visitor.js b/lib/visitor.js index c65bda2d..62a610e8 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -206,27 +206,20 @@ class Visitor { } } - buffer.addIndented(() => { + buffer.addIndented(function() { for (const e of enums) { buffer.add(`static ${e.name} = ${propertyToInlineEnumName(clean, e.name)}`) file.addInlineEnum(clean, name, e.name, csnToEnumPairs(e, {unwrapVals: true})) } buffer.add('static actions: {') - buffer.addIndented(function() { - for (const [aname, action] of Object.entries(entity.actions ?? {})) { - buffer.add( - SourceFile.stringifyLambda({ - name: aname, - parameters: this.#stringifyFunctionParams(action.params, file), - returns: action.returns ? this.resolver.resolveAndRequire(action.returns, file).typeName : 'any' - //initialiser: `undefined as unknown as typeof ${clean}.${aname}`, - }) - ) - } - }.bind(this)) - }) - buffer.add('}') // end of actions - + const actions = Object.entries(entity.actions ?? {}) + buffer.addIndented(actions.map(([aname, action]) => SourceFile.stringifyLambda({ + name: aname, + parameters: this.#stringifyFunctionParams(action.params, file), + returns: action.returns ? this.resolver.resolveAndRequire(action.returns, file).typeName : 'any' + }))) + buffer.add('}') // end of actions + }.bind(this)) buffer.outdent() buffer.add('};') // end of generated class buffer.outdent() @@ -409,14 +402,14 @@ class Visitor { const buffer = file.events.buffer buffer.add('// event') buffer.add(`export class ${clean} {`) - buffer.addIndented(() => { + buffer.addIndented(function() { const propOpt = this.options.propertiesOptional this.options.propertiesOptional = false for (const [ename, element] of Object.entries(event.elements ?? {})) { this.visitElement(ename, element, file, buffer) } this.options.propertiesOptional = propOpt - }) + }.bind(this)) buffer.add('}') } From a575214edc8ad078de0c13bedf6f1679fc74b6b9 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Wed, 21 Feb 2024 17:56:54 +0100 Subject: [PATCH 3/9] Add indented block --- lib/file.js | 12 +++++ lib/visitor.js | 93 ++++++++++++++++++--------------------- test/unit/actions.test.js | 1 - 3 files changed, 56 insertions(+), 50 deletions(-) diff --git a/lib/file.js b/lib/file.js index b6dd2e69..3afac082 100644 --- a/lib/file.js +++ b/lib/file.js @@ -495,6 +495,18 @@ class Buffer { } this.outdent() } + + /** + * Adds an element to a buffer with one level of indent and and opener and closer surrounding it. + * @param {string} opener the string to put before the indent + * @param {string} content the content to indent (see {@link addIndented}) + * @param {string} closer the string to put after the indent + */ + addIndentedBlock(opener, content, closer) { + this.add(opener) + this.addIndented(content) + this.add(closer) + } } /** diff --git a/lib/visitor.js b/lib/visitor.js index 62a610e8..f95a6dc0 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -171,59 +171,54 @@ class Visitor { this.contexts.push({ entity: name }) // CLASS ASPECT - buffer.add(`export function ${identAspect(clean)} object>(Base: TBase) {`) - buffer.indent() - buffer.add(`return class ${clean} extends Base {`) - buffer.indent() - - const enums = [] - const exclusions = new Set(entity.projection?.excluding ?? []) - const elements = Object.entries(entity.elements ?? {}).filter(([ename]) => !exclusions.has(ename)) - for (const [ename, element] of elements) { - this.visitElement(ename, element, file, buffer) - - // make foreign keys explicit - if ('target' in element) { - // lookup in cds.definitions can fail for inline structs. - // We don't really have to care for this case, as keys from such structs are _not_ propagated to - // the containing entity. - for (const [kname, kelement] of this.#keys(element.target)) { - if (this.resolver.getMaxCardinality(element) === 1) { - const foreignKey = `${ename}_${kname}` - if (Object.hasOwn(entity.elements, foreignKey)) { - this.logger.error(`Attempting to generate a foreign key reference called '${foreignKey}' in type definition for entity ${name}. But a property of that name is already defined explicitly. Consider renaming that property.`) - } else { - kelement.isRefNotNull = !!element.notNull || !!element.key - this.visitElement(foreignKey, kelement, file, buffer) + buffer.addIndentedBlock(`export function ${identAspect(clean)} object>(Base: TBase) {`, function () { + buffer.addIndentedBlock(`return class ${clean} extends Base {`, function () { + const enums = [] + const exclusions = new Set(entity.projection?.excluding ?? []) + const elements = Object.entries(entity.elements ?? {}).filter(([ename]) => !exclusions.has(ename)) + for (const [ename, element] of elements) { + this.visitElement(ename, element, file, buffer) + + // make foreign keys explicit + if ('target' in element) { + // lookup in cds.definitions can fail for inline structs. + // We don't really have to care for this case, as keys from such structs are _not_ propagated to + // the containing entity. + for (const [kname, kelement] of this.#keys(element.target)) { + if (this.resolver.getMaxCardinality(element) === 1) { + const foreignKey = `${ename}_${kname}` + if (Object.hasOwn(entity.elements, foreignKey)) { + this.logger.error(`Attempting to generate a foreign key reference called '${foreignKey}' in type definition for entity ${name}. But a property of that name is already defined explicitly. Consider renaming that property.`) + } else { + kelement.isRefNotNull = !!element.notNull || !!element.key + this.visitElement(foreignKey, kelement, file, buffer) + } + } } } - } - } - // store inline enums for later handling, as they have to go into one common "static elements" wrapper - if (isInlineEnumType(element, this.csn.xtended)) { - enums.push(element) - } - } + // store inline enums for later handling, as they have to go into one common "static elements" wrapper + if (isInlineEnumType(element, this.csn.xtended)) { + enums.push(element) + } + } - buffer.addIndented(function() { - for (const e of enums) { - buffer.add(`static ${e.name} = ${propertyToInlineEnumName(clean, e.name)}`) - file.addInlineEnum(clean, name, e.name, csnToEnumPairs(e, {unwrapVals: true})) - } - buffer.add('static actions: {') - const actions = Object.entries(entity.actions ?? {}) - buffer.addIndented(actions.map(([aname, action]) => SourceFile.stringifyLambda({ - name: aname, - parameters: this.#stringifyFunctionParams(action.params, file), - returns: action.returns ? this.resolver.resolveAndRequire(action.returns, file).typeName : 'any' - }))) - buffer.add('}') // end of actions - }.bind(this)) - buffer.outdent() - buffer.add('};') // end of generated class - buffer.outdent() - buffer.add('}') // end of aspect + buffer.addIndented(function() { + for (const e of enums) { + buffer.add(`static ${e.name} = ${propertyToInlineEnumName(clean, e.name)}`) + file.addInlineEnum(clean, name, e.name, csnToEnumPairs(e, {unwrapVals: true})) + } + const actions = Object.entries(entity.actions ?? {}) + buffer.addIndentedBlock('static actions: {', + actions.map(([aname, action]) => SourceFile.stringifyLambda({ + name: aname, + parameters: this.#stringifyFunctionParams(action.params, file), + returns: action.returns ? this.resolver.resolveAndRequire(action.returns, file).typeName : 'any' + })) + , '}') // end of actions + }.bind(this)) + }.bind(this), '};') // end of generated class + }.bind(this), '}') // end of aspect // CLASS WITH ADDED ASPECTS file.addImport(baseDefinitions.path) diff --git a/test/unit/actions.test.js b/test/unit/actions.test.js index 95f87490..52b475d9 100644 --- a/test/unit/actions.test.js +++ b/test/unit/actions.test.js @@ -94,7 +94,6 @@ describe('Actions', () => { const actions = astw.getAspectProperty('_EAspect', 'actions') expect(actions.modifiers.some(check.isStatic)).toBeTruthy() - // mainly make sure $self parameter is not present at all checkFunction(actions.type.members.find(fn => fn.name === 's1'), { callCheck: signature => check.isAny(signature), From e3cef68dfad10a3fb9de4571eb7e2e68ede22533 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Wed, 21 Feb 2024 19:08:03 +0100 Subject: [PATCH 4/9] Cleanup --- lib/csn.js | 4 ++- lib/file.js | 5 ++- lib/visitor.js | 93 ++++++++++++++++++++++++++++---------------------- 3 files changed, 60 insertions(+), 42 deletions(-) diff --git a/lib/csn.js b/lib/csn.js index 46424339..3e563015 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -234,10 +234,12 @@ function amendCSN(csn) { */ const isView = entity => entity.query && !entity.projection +const isDraftEnabled = entity => entity['@odata.draft.enabled'] === true + /** * @see isView * Unresolved entities have to be looked up from inferred csn. */ const isUnresolved = entity => entity._unresolved === true -module.exports = { amendCSN, isView, isUnresolved, propagateForeignKeys } \ No newline at end of file +module.exports = { amendCSN, isView, isDraftEnabled, isUnresolved, propagateForeignKeys } \ No newline at end of file diff --git a/lib/file.js b/lib/file.js index 3afac082..fc6c69ce 100644 --- a/lib/file.js +++ b/lib/file.js @@ -93,10 +93,13 @@ class Library extends File { * Source file containing several buffers. */ class SourceFile extends File { + /** + * @param {string | Path} path + */ constructor(path) { super() /** @type {Path} */ - this.path = new Path(path.split('.')) + this.path = path instanceof Path ? path : new Path(path.split('.')) /** @type {Object} */ this.imports = {} /** @type {Buffer} */ diff --git a/lib/visitor.js b/lib/visitor.js index f95a6dc0..4f5b273c 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -2,9 +2,9 @@ const util = require('./util') -const { amendCSN, isView, isUnresolved, propagateForeignKeys } = require('./csn') +const { amendCSN, isView, isUnresolved, propagateForeignKeys, isDraftEnabled } = require('./csn') // eslint-disable-next-line no-unused-vars -const { SourceFile, baseDefinitions, Buffer } = require('./file') +const { SourceFile, baseDefinitions, Buffer, Path } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') const { Resolver } = require('./components/resolver') const { Logger } = require('./logging') @@ -46,6 +46,38 @@ const defaults = { inlineDeclarations: 'flat' } +// TODO: having the repository pattern in place we can separate (some of) the printing logic from the visitor. +// Most of it hinges primarily on resolving specific files. We can now pass the repository and the resolver to a printer. +class FileRepository { + #files = {} + + /** + * @param {string} name + * @param {SourceFile} file + */ + add(name, file) { + this.#files[name] = file + } + + /** + * Determines the file corresponding to the namespace. + * If no such file exists yet, it is created first. + * @param {string | Path} path the name of the namespace (foo.bar.baz) + * @returns {SourceFile} the file corresponding to that namespace name + */ + getNamespaceFile(path) { + const key = path instanceof Path ? path.asIdentifier() : path + return (this.#files[key] ??= new SourceFile(path)) + } + + /** + * @returns {SourceFile[]} + */ + getFiles() { + return Object.values(this.#files) + } +} + class Visitor { /** * Gathers all files that are supposed to be written to @@ -54,7 +86,7 @@ class Visitor { * @returns {File[]} a full list of files to be written */ getWriteoutFiles() { - return Object.values(this.files).concat(this.resolver.getUsedLibraries()) + return this.fileRepository.getFiles().concat(this.resolver.getUsedLibraries()) } /** @@ -74,9 +106,9 @@ class Visitor { /** @type {Resolver} */ this.resolver = new Resolver(this) - /** @type {Object} */ - this.files = {} - this.files[baseDefinitions.path.asIdentifier()] = baseDefinitions + /** @type {FileRepository} */ + this.fileRepository = new FileRepository() + this.fileRepository.add(baseDefinitions.path.asIdentifier(), baseDefinitions) this.inlineDeclarationResolver = this.options.inlineDeclarations === 'structured' ? new StructuredInlineDeclarationResolver(this) @@ -85,16 +117,6 @@ class Visitor { this.visitDefinitions() } - /** - * Determines the file corresponding to the namespace. - * If no such file exists yet, it is created first. - * @param {string} path the name of the namespace (foo.bar.baz) - * @returns {SourceFile} the file corresponding to that namespace name - */ - getNamespaceFile(path) { - return (this.files[path] ??= new SourceFile(path)) - } - /** * Visits all definitions within the CSN definitions. */ @@ -163,7 +185,7 @@ class Visitor { #aspectify(name, entity, buffer, cleanName = undefined) { const clean = cleanName ?? this.resolver.trimNamespace(name) const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const file = this.fileRepository.getNamespaceFile(ns) const identSingular = (name) => name const identAspect = (name) => `_${name}Aspect` @@ -185,7 +207,7 @@ class Visitor { // We don't really have to care for this case, as keys from such structs are _not_ propagated to // the containing entity. for (const [kname, kelement] of this.#keys(element.target)) { - if (this.resolver.getMaxCardinality(element) === 1) { + if (this.resolver.getMaxCardinality(element) === 1) { // FIXME: kelement? const foreignKey = `${ename}_${kname}` if (Object.hasOwn(entity.elements, foreignKey)) { this.logger.error(`Attempting to generate a foreign key reference called '${foreignKey}' in type definition for entity ${name}. But a property of that name is already defined explicitly. Consider renaming that property.`) @@ -243,12 +265,8 @@ class Visitor { this.contexts.pop() } - #isDraftEnabled(entity) { - return entity['@odata.draft.enabled'] === true - } - #staticClassContents(clean, entity) { - return this.#isDraftEnabled(entity) ? [`static drafts: typeof ${clean}`] : [] + return isDraftEnabled(entity) ? [`static drafts: typeof ${clean}`] : [] } #printEntity(name, entity) { @@ -256,7 +274,7 @@ class Visitor { const overrideNameProperty = (clazz, content) => `Object.defineProperty(${clazz}, 'name', { value: '${content}' })` const clean = this.resolver.trimNamespace(name) const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const file = this.fileRepository.getNamespaceFile(ns) // entities are expected to be in plural anyway, so we would favour the regular name. // If the user decides to pass a @plural annotation, that gets precedence over the regular name. let plural = this.resolver.trimNamespace(util.getPluralAnnotation(entity) ? util.plural4(entity, false) : name) @@ -342,7 +360,7 @@ class Visitor { // FIXME: mostly duplicate of printAction -> reuse this.logger.debug(`Printing function ${name}:\n${JSON.stringify(func, null, 2)}`) const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const file = this.fileRepository.getNamespaceFile(ns) const params = this.#stringifyFunctionParams(func.params, file) const returns = this.resolver.visitor.inlineDeclarationResolver.getPropertyDatatype( this.resolver.resolveAndRequire(func.returns, file) @@ -353,7 +371,7 @@ class Visitor { #printAction(name, action) { this.logger.debug(`Printing action ${name}:\n${JSON.stringify(action, null, 2)}`) const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const file = this.fileRepository.getNamespaceFile(ns) const params = this.#stringifyFunctionParams(action.params, file) const returns = this.resolver.visitor.inlineDeclarationResolver.getPropertyDatatype( this.resolver.resolveAndRequire(action.returns, file) @@ -363,9 +381,8 @@ class Visitor { #printType(name, type) { this.logger.debug(`Printing type ${name}:\n${JSON.stringify(type, null, 2)}`) - const clean = this.resolver.trimNamespace(name) - const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const [ns, clean] = this.resolver.untangle(name) + const file = this.fileRepository.getNamespaceFile(ns) if ('enum' in type) { file.addEnum(name, clean, csnToEnumPairs(type)) } else { @@ -377,9 +394,8 @@ class Visitor { #printAspect(name, aspect) { this.logger.debug(`Printing aspect ${name}`) - const clean = this.resolver.trimNamespace(name) - const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const [ns, clean] = this.resolver.untangle(name) + const file = this.fileRepository.getNamespaceFile(ns) // aspects are technically classes and can therefore be added to the list of defined classes. // Still, when using them as mixins for a class, they need to already be defined. // So we separate them into another buffer which is printed before the classes. @@ -390,28 +406,25 @@ class Visitor { #printEvent(name, event) { this.logger.debug(`Printing event ${name}`) - const clean = this.resolver.trimNamespace(name) - const ns = this.resolver.resolveNamespace(name.split('.')) - const file = this.getNamespaceFile(ns) + const [ns, clean] = this.resolver.untangle(name) + const file = this.fileRepository.getNamespaceFile(ns) file.addEvent(clean, name) const buffer = file.events.buffer buffer.add('// event') - buffer.add(`export class ${clean} {`) - buffer.addIndented(function() { + buffer.addIndentedBlock(`export class ${clean} {`, function() { const propOpt = this.options.propertiesOptional this.options.propertiesOptional = false for (const [ename, element] of Object.entries(event.elements ?? {})) { this.visitElement(ename, element, file, buffer) } this.options.propertiesOptional = propOpt - }.bind(this)) - buffer.add('}') + }.bind(this), '}') } #printService(name, service) { this.logger.debug(`Printing service ${name}:\n${JSON.stringify(service, null, 2)}`) const ns = this.resolver.resolveNamespace(name) - const file = this.getNamespaceFile(ns) + const file = this.fileRepository.getNamespaceFile(ns) // service.name is clean of namespace file.services.buffer.add(`export default { name: '${service.name}' }`) file.addService(service.name) From ee9362b3b0170f956e982116d9d8071cebf644ed Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Wed, 21 Feb 2024 19:32:50 +0100 Subject: [PATCH 5/9] Move FileRepository to file.js --- lib/components/enum.js | 10 +++------- lib/file.js | 33 +++++++++++++++++++++++++++++++++ lib/visitor.js | 34 +--------------------------------- 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/lib/components/enum.js b/lib/components/enum.js index b40cc698..9d14703f 100644 --- a/lib/components/enum.js +++ b/lib/components/enum.js @@ -29,13 +29,9 @@ const { normalise } = require('./identifier') function printEnum(buffer, name, kvs, options = {}) { const opts = {...{export: true}, ...options} buffer.add('// enum') - buffer.add(`${opts.export ? 'export ' : ''}const ${name} = {`) - buffer.indent() - for (const [k, v] of kvs) { - buffer.add(`${normalise(k)}: ${v},`) - } - buffer.outdent() - buffer.add('} as const;') + buffer.addIndentedBlock(`${opts.export ? 'export ' : ''}const ${name} = {`, () => + kvs.forEach(([k, v]) => buffer.add(`${normalise(k)}: ${v},`)) + , '} as const;') buffer.add(`${opts.export ? 'export ' : ''}type ${name} = ${stringifyEnumType(kvs)}`) buffer.add('') } diff --git a/lib/file.js b/lib/file.js index fc6c69ce..3e6185a7 100644 --- a/lib/file.js +++ b/lib/file.js @@ -578,6 +578,38 @@ class Path { } } +// TODO: having the repository pattern in place we can separate (some of) the printing logic from the visitor. +// Most of it hinges primarily on resolving specific files. We can now pass the repository and the resolver to a printer. +class FileRepository { + #files = {} + + /** + * @param {string} name + * @param {SourceFile} file + */ + add(name, file) { + this.#files[name] = file + } + + /** + * Determines the file corresponding to the namespace. + * If no such file exists yet, it is created first. + * @param {string | Path} path the name of the namespace (foo.bar.baz) + * @returns {SourceFile} the file corresponding to that namespace name + */ + getNamespaceFile(path) { + const key = path instanceof Path ? path.asIdentifier() : path + return (this.#files[key] ??= new SourceFile(path)) + } + + /** + * @returns {SourceFile[]} + */ + getFiles() { + return Object.values(this.#files) + } +} + /** * Base definitions used throughout the typing process, * such as Associations and Compositions. @@ -648,6 +680,7 @@ module.exports = { Library, Buffer, File, + FileRepository, SourceFile, Path, writeout, diff --git a/lib/visitor.js b/lib/visitor.js index 4f5b273c..37febd3f 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -4,7 +4,7 @@ const util = require('./util') const { amendCSN, isView, isUnresolved, propagateForeignKeys, isDraftEnabled } = require('./csn') // eslint-disable-next-line no-unused-vars -const { SourceFile, baseDefinitions, Buffer, Path } = require('./file') +const { SourceFile, FileRepository, baseDefinitions, Buffer } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') const { Resolver } = require('./components/resolver') const { Logger } = require('./logging') @@ -46,38 +46,6 @@ const defaults = { inlineDeclarations: 'flat' } -// TODO: having the repository pattern in place we can separate (some of) the printing logic from the visitor. -// Most of it hinges primarily on resolving specific files. We can now pass the repository and the resolver to a printer. -class FileRepository { - #files = {} - - /** - * @param {string} name - * @param {SourceFile} file - */ - add(name, file) { - this.#files[name] = file - } - - /** - * Determines the file corresponding to the namespace. - * If no such file exists yet, it is created first. - * @param {string | Path} path the name of the namespace (foo.bar.baz) - * @returns {SourceFile} the file corresponding to that namespace name - */ - getNamespaceFile(path) { - const key = path instanceof Path ? path.asIdentifier() : path - return (this.#files[key] ??= new SourceFile(path)) - } - - /** - * @returns {SourceFile[]} - */ - getFiles() { - return Object.values(this.#files) - } -} - class Visitor { /** * Gathers all files that are supposed to be written to From 46bd7f06e54d34d99b0a737d22814b7b63ca3548 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Thu, 22 Feb 2024 11:22:35 +0100 Subject: [PATCH 6/9] Disable integration tests --- test/int.jest.config.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/int.jest.config.js b/test/int.jest.config.js index dcce79db..0cab6c03 100644 --- a/test/int.jest.config.js +++ b/test/int.jest.config.js @@ -1,10 +1,13 @@ // as xmake does not allow requests towards external resources (github.com), // we need to split tests referring to cap-cloud-samples into their own // test suite. +// FIXME: reneable integration tests or merge them with unit tests +/* module.exports = { displayName: "integration", rootDir: '..', testRegex: "./unit/.*.test.js", globalSetup: "/test/integration/setup.js", cache: false -} \ No newline at end of file +} +*/ \ No newline at end of file From bf81ae29b8a59764a34261b990e36278a1a57648 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Thu, 22 Feb 2024 17:57:14 +0100 Subject: [PATCH 7/9] Only clean test directory once --- lib/file.js | 2 +- lib/visitor.js | 2 +- test/int.jest.config.js | 2 +- test/unit.jest.config.js | 3 ++- test/unit/enum.test.js | 6 +++--- test/unit/setup.js | 9 +++++++++ test/util.js | 5 +++-- 7 files changed, 20 insertions(+), 9 deletions(-) create mode 100644 test/unit/setup.js diff --git a/lib/file.js b/lib/file.js index e69356be..e26bfe29 100644 --- a/lib/file.js +++ b/lib/file.js @@ -598,7 +598,7 @@ class FileRepository { * @returns {SourceFile} the file corresponding to that namespace name */ getNamespaceFile(path) { - const key = path instanceof Path ? path.asIdentifier() : path + const key = path instanceof Path ? path.asNamespace() : path return (this.#files[key] ??= new SourceFile(path)) } diff --git a/lib/visitor.js b/lib/visitor.js index 37febd3f..96a3fe73 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -76,7 +76,7 @@ class Visitor { /** @type {FileRepository} */ this.fileRepository = new FileRepository() - this.fileRepository.add(baseDefinitions.path.asIdentifier(), baseDefinitions) + this.fileRepository.add(baseDefinitions.path.asNamespace(), baseDefinitions) this.inlineDeclarationResolver = this.options.inlineDeclarations === 'structured' ? new StructuredInlineDeclarationResolver(this) diff --git a/test/int.jest.config.js b/test/int.jest.config.js index 0cab6c03..9cbaad5a 100644 --- a/test/int.jest.config.js +++ b/test/int.jest.config.js @@ -6,7 +6,7 @@ module.exports = { displayName: "integration", rootDir: '..', - testRegex: "./unit/.*.test.js", + testRegex: "./integration/.*.test.js", globalSetup: "/test/integration/setup.js", cache: false } diff --git a/test/unit.jest.config.js b/test/unit.jest.config.js index b1a19ca6..24e478d8 100644 --- a/test/unit.jest.config.js +++ b/test/unit.jest.config.js @@ -2,5 +2,6 @@ module.exports = { displayName: "unit", rootDir: "..", testRegex: "./unit/.*.test.js", - cache: false + cache: false, + globalSetup: "/test/unit/setup.js", } \ No newline at end of file diff --git a/test/unit/enum.test.js b/test/unit/enum.test.js index bbcb863f..d683e7b5 100644 --- a/test/unit/enum.test.js +++ b/test/unit/enum.test.js @@ -8,7 +8,7 @@ const { locations, prepareUnitTest } = require('../util') describe('Enum Action Parameters', () => { let astw - beforeAll(async () => astw = (await prepareUnitTest('enums/actions.cds', locations.testOutput('enums_test_actions'))).astw) + beforeAll(async () => astw = (await prepareUnitTest('enums/actions.cds', locations.testOutput('enums_test'))).astw) test('Coalescing Assignment Present', () => { const actions = astw.getAspectProperty('_FoobarAspect', 'actions') @@ -35,7 +35,7 @@ describe('Nested Enums', () => { let astw beforeAll(async () => { - const paths = (await prepareUnitTest('enums/nested.cds', locations.testOutput('enums_test_nested'))).paths + const paths = (await prepareUnitTest('enums/nested.cds', locations.testOutput('enums_test'))).paths astw = await JSASTWrapper.initialise(path.join(paths[1], 'index.js')) }) @@ -53,7 +53,7 @@ describe('Nested Enums', () => { describe('Enum Types', () => { let astw - beforeAll(async () => astw = (await prepareUnitTest('enums/model.cds', locations.testOutput('enums_test_model'))).astw) + beforeAll(async () => astw = (await prepareUnitTest('enums/model.cds', locations.testOutput('enums_test'))).astw) describe('Anonymous', () => { describe('Within type Definition', () => { diff --git a/test/unit/setup.js b/test/unit/setup.js new file mode 100644 index 00000000..f1544319 --- /dev/null +++ b/test/unit/setup.js @@ -0,0 +1,9 @@ +const { fs } = require("@sap/cds/lib/utils/cds-utils") +const util = require('../util') + +module.exports = () => { + const base = util.locations.testOutputBase + try { + fs.unlinkSync(base) + } catch (e) { /* also fails on permissions, but still ignore */ } +} \ No newline at end of file diff --git a/test/util.js b/test/util.js index d0582d6f..835c09c1 100644 --- a/test/util.js +++ b/test/util.js @@ -298,6 +298,7 @@ const resolveAliases = (file, resolves) => { } const locations = { + testOutputBase: path.normalize(`${os.tmpdir}/type-gen/test/output/`), testOutput: (suffix) => { const dir = path.normalize(`${os.tmpdir}/type-gen/test/output/${suffix}`) console.log(`preparing test output directory: ${dir}`) @@ -320,9 +321,9 @@ const cds2ts = async (cdsFile, options = {}) => typer.compileFromFile( options ) -async function prepareUnitTest(model, outputDirectory, typerOptions = {}, fileSelector = paths => paths[1]) { +async function prepareUnitTest(model, outputDirectory, typerOptions = {}, fileSelector = paths => paths.find(p => !p.endsWith('_'))) { const options = {...{ outputDirectory: outputDirectory, inlineDeclarations: 'structured' }, ...typerOptions} - await unlink(outputDirectory).catch(() => {}) + //await unlink(outputDirectory).catch(() => {}) const paths = await cds2ts(model, options) // eslint-disable-next-line no-console .catch((err) => console.error(err)) From b62ace1235c8ab1f6051f9ce3e66fe075c17ade0 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Thu, 22 Feb 2024 17:58:31 +0100 Subject: [PATCH 8/9] Appease linter --- test/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/util.js b/test/util.js index 835c09c1..5e6578fb 100644 --- a/test/util.js +++ b/test/util.js @@ -1,6 +1,6 @@ /* eslint-disable no-console */ const fs = require('fs') -const { unlink } = require('fs').promises +// const { unlink } = require('fs').promises const path = require('path') const { Logger } = require('../lib/logging') const { fail } = require('assert') From 11d3f3d287a168a8c17d14da834952c8050aa384 Mon Sep 17 00:00:00 2001 From: Daniel O'Grady Date: Thu, 22 Feb 2024 18:01:24 +0100 Subject: [PATCH 9/9] Disable integration tests --- .github/workflows/test.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 46fa4a07..594c5658 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,7 +26,7 @@ jobs: npm install npm run test - - name: Run Integration Tests - run: | - npm install - npm run test:integration + # - name: Run Integration Tests + # run: | + # npm install + # npm run test:integration