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 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/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 9d8e6651..e26bfe29 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} */ @@ -480,6 +483,33 @@ class Buffer { add(part) { this.parts.push(this.currentIndent + part) } + + /** + * Adds an element to the buffer with one level of indent. + * @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() + if (typeof part === 'function') { + part() + } else if (Array.isArray(part)) { + part.forEach(p => this.add(p)) + } + 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) + } } /** @@ -548,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.asNamespace() : 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. @@ -618,6 +680,7 @@ module.exports = { Library, Buffer, File, + FileRepository, SourceFile, Path, writeout, diff --git a/lib/visitor.js b/lib/visitor.js index e2d7589b..96a3fe73 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, FileRepository, baseDefinitions, Buffer } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') const { Resolver } = require('./components/resolver') const { Logger } = require('./logging') @@ -54,7 +54,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 +74,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.asNamespace(), baseDefinitions) this.inlineDeclarationResolver = this.options.inlineDeclarations === 'structured' ? new StructuredInlineDeclarationResolver(this) @@ -85,16 +85,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 +153,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` @@ -171,66 +161,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) { // 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.`) + } 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) - } - } - - 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.add('}') // end of actions + // 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.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) @@ -255,12 +233,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) { @@ -268,7 +242,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) @@ -354,7 +328,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) @@ -365,7 +339,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) @@ -375,9 +349,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 { @@ -389,9 +362,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. @@ -402,28 +374,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.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.add('}') + 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), '}') } #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) diff --git a/test/int.jest.config.js b/test/int.jest.config.js index dcce79db..9cbaad5a 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", + testRegex: "./integration/.*.test.js", globalSetup: "/test/integration/setup.js", cache: false -} \ No newline at end of file +} +*/ \ No newline at end of file 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/actions.test.js b/test/unit/actions.test.js index 9b794529..02b22de4 100644 --- a/test/unit/actions.test.js +++ b/test/unit/actions.test.js @@ -91,7 +91,6 @@ describe('Actions', () => { test('Bound Expecting $self Arguments', async () => { const actions = astwBound.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), 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..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') @@ -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))