From 6e547c33fad2356c2825e493a038546414d5c556 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 5 Oct 2024 17:35:27 +0200 Subject: [PATCH 1/9] refactor: overhaul/fix creation of `drafts` properties in entities --- eslint.config.js | 3 + lib/csn.js | 250 ++++++++---------- lib/visitor.js | 18 +- test/unit/draft.test.js | 69 ++--- .../files/draft/catalog-service-error.cds | 10 + test/unit/files/draft/catalog-service.cds | 3 +- test/unit/files/draft/data-model.cds | 7 +- test/unit/files/draft/model.cds | 51 ---- test/util.js | 3 +- 9 files changed, 152 insertions(+), 262 deletions(-) create mode 100644 test/unit/files/draft/catalog-service-error.cds delete mode 100644 test/unit/files/draft/model.cds diff --git a/eslint.config.js b/eslint.config.js index 0cca5c24..0f74f434 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -13,6 +13,9 @@ module.exports = [ ...require('globals').node } }, + env: { + jest: true + }, files: ['**/*.js'], plugins: { '@stylistic/js': require('@stylistic/eslint-plugin-js') diff --git a/lib/csn.js b/lib/csn.js index 837951fe..93c0f714 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -1,4 +1,8 @@ -const annotation = '@odata.draft.enabled' +const { LOG } = require('./logging') + +const DRAFT_ENABLED_ANNO = '@odata.draft.enabled' +/** @type {string[]} */ +const draftEnabledEntities = [] /** @typedef {import('./typedefs').resolver.CSN} CSN */ /** @typedef {import('./typedefs').resolver.EntityCSN} EntityCSN */ @@ -40,9 +44,9 @@ const isUnresolved = entity => entity._unresolved === true const isCsnAny = entity => entity?.constructor?.name === 'any' /** - * @param {EntityCSN} entity - the entity + * @param {string} fq - the fqn of an entity */ -const isDraftEnabled = entity => entity['@odata.draft.enabled'] === true +const isDraftEnabled = fq => draftEnabledEntities.includes(fq) /** * @param {EntityCSN} entity - the entity @@ -87,183 +91,144 @@ const getProjectionTarget = entity => isProjection(entity) ? entity.projection?.from?.ref?.[0] : undefined -class DraftUnroller { - /** @type {Set} */ - #positives = new Set() - /** @type {{[key: string]: boolean}} */ - #draftable = {} - /** @type {{[key: string]: string}} */ - #projections = {} +class DraftEnabledEntityCollector { /** @type {EntityCSN[]} */ - #entities = [] + #draftRoots = [] + /** @type {string[]} */ + #serviceNames = [] /** @type {CSN | undefined} */ #csn - set csn(c) { - this.#csn = c - if (c === undefined) return - this.#entities = Object.values(c.definitions) - this.#projections = this.#entities.reduce((pjs, entity) => { - if (isProjection(entity)) { - // @ts-ignore - we know that entity is a projection here - pjs[entity.name] = getProjectionTarget(entity) - } - return pjs - }, {}) - } - get csn() { return this.#csn } + #compileError = false /** - * @param {EntityCSN | string} entityOrFq - entity to set draftable annotation for. - * @param {boolean} value - whether the entity is draftable. + * @returns {string[]} */ - #setDraftable(entityOrFq, value) { - const entity = typeof entityOrFq === 'string' - ? this.#getDefinition(entityOrFq) - : entityOrFq - if (!entity) return // inline definition -- not found in definitions - entity[annotation] = value - this.#draftable[entity.name] = value - if (value) { - this.#positives.add(entity.name) - } else { - this.#positives.delete(entity.name) - } + #getServiceNames() { + return Object.values(this.#csn?.definitions ?? {}).filter(d => d.kind === 'service').map(d => d.name) } /** - * @param {EntityCSN | string} entityOrFq - entity to look draftability up for. - * @returns {boolean} + * @returns {EntityCSN[]} */ - #getDraftable(entityOrFq) { - const entity = typeof entityOrFq === 'string' - ? this.#getDefinition(entityOrFq) - : entityOrFq - // assert(typeof entity !== 'string') - const name = entity?.name ?? entityOrFq - // @ts-expect-error - .name not being present means entityOrFq is a string, so name is always a string and therefore a valid index - return this.#draftable[name] ??= this.#propagateInheritance(entity) + #collectDraftRoots() { + return Object.values(this.#csn?.definitions ?? {}).filter( + d => isEntity(d) && this.#isDraftEnabled(d) && this.#isPartOfAnyService(d.name) + ) } /** - * FIXME: could use EntityRepository here - * @param {string} name - name of the entity. - * @returns {EntityCSN} + * @param {string} entityName - entity to check + * @returns {boolean} `true` if entity is part an service */ - // @ts-expect-error - poor man's #getDefinitionOrThrow. We are always sure name is a valid key - #getDefinition(name) { return this.csn?.definitions[name] } - - /** - * Propagate draft annotations through inheritance (includes). - * The latest annotation through the inheritance chain "wins". - * Annotations on the entity itself are always queued last, so they will always be decisive over ancestors. - * @param {EntityCSN | undefined} entity - entity to pull draftability from its parents. - */ - #propagateInheritance(entity) { - if (!entity) return - /** @type {(boolean | undefined)[]} */ - const annotations = (entity.includes ?? []).map(parent => this.#getDraftable(parent)) - annotations.push(entity[annotation]) - this.#setDraftable(entity, annotations.filter(a => a !== undefined).at(-1) ?? false) + #isPartOfAnyService(entityName) { + return this.#serviceNames.some(s => entityName.startsWith(s)) } /** - * Propagate draft-enablement through projections. + * Collect all entities that are transitively reachable via compositions from `entity` into `draftNodes`. + * Check that no entity other than the root node has `@odata.draft.enabled` + * @param {EntityCSN} entity - + * @param {string} entityName - + * @param {EntityCSN} rootEntity - root entity where composition traversal started. + * @param {Record} draftEntities - Dictionary of entitys */ - #propagateProjections() { - /** - * @param {string} from - entity to propagate draftability from. - * @param {string} to - entity to propagate draftability to. - */ - const propagate = (from, to) => { - do { - this.#setDraftable(to, this.#getDraftable(to) || this.#getDraftable(from)) - from = to - to = this.#projections[to] - } while (to) - } + #collectDraftNodesInto(entity, entityName, rootEntity, draftEntities) { + draftEntities[entityName] = entity - for (let [projection, target] of Object.entries(this.#projections)) { - propagate(projection, target) - propagate(target, projection) + for (const elem of Object.values(entity.elements ?? {})) { + if (!elem.target || elem.type !== 'cds.Composition') continue + + const draftNode = this.#csn?.definitions[elem.target] + const draftNodeName = elem.target + + if (!draftNode) { + throw new Error(`Expecting target to be resolved: ${JSON.stringify(elem, null, 2)}`) + } + + if (!this.#isPartOfAnyService(draftNodeName)) { + LOG.warn(`Ignoring draft node for composition target ${draftNodeName} because it is not part of a service`) + continue + } + + if (draftNode !== rootEntity && this.#isDraftEnabled(draftNode)) { + this.#compileError = true + LOG.error(`Composition in draft-enabled entity can't lead to another entity with "@odata.draft.enabled" (in entity: "${entityName}"/element: ${elem.name})!`) + delete draftEntities[draftNodeName] + continue + } + + if (!this.#isDraftEnabled(draftNode) && !draftEntities[draftNodeName]) { + this.#collectDraftNodesInto(draftNode, draftNodeName, rootEntity, draftEntities) + } } } /** - * If an entity E is draftable and contains any composition of entities, - * then those entities also become draftable. Recursively. - * @param {EntityCSN} entity - entity to propagate all compositions from. + * @param {EntityCSN} entity - entity to check + * @returns {boolean} */ - #propagateCompositions(entity) { - if (!this.#getDraftable(entity)) return - - for (const comp of Object.values(entity.compositions ?? {})) { - const target = this.#getDefinition(comp.target) - const current = this.#getDraftable(target) - if (!current) { - this.#setDraftable(target, true) - this.#propagateCompositions(target) - } - } + #isDraftEnabled(entity) { + return entity[DRAFT_ENABLED_ANNO] === true } /** @param {CSN} csn - the full csn */ - unroll(csn) { - this.csn = csn + run(csn) { + if (!csn) return - // inheritance - for (const entity of this.#entities) { - this.#propagateInheritance(entity) - } + this.#csn = csn + this.#serviceNames = this.#getServiceNames() + this.#draftRoots = this.#collectDraftRoots() - // transitivity through compositions - // we have to do this in a second pass, as we only now know which entities are draft-enables themselves - for (const entity of this.#entities) { - this.#propagateCompositions(entity) - } + for (const draftRoot of this.#draftRoots) { + /** @type {Record} */ + const draftEntities = {} + this.#collectDraftNodesInto(draftRoot, draftRoot.name, draftRoot, draftEntities) - this.#propagateProjections() + for (const draftNode of Object.values(draftEntities)) { + draftEnabledEntities.push(draftNode.name) + } + } + if (this.#compileError) throw new Error('Compilation of model failed') } } // note to self: following doc uses ＠ homoglyph instead of @, as the latter apparently has special semantics in code listings /** - * We are unrolling the @odata.draft.enabled annotations into related entities manually. - * This includes three scenarios: + * We collect all entities that are draft enabled. + * (@see `@sap/cds-compiler/lib/transform/draft/db.js#generateDraft`) * - * (a) aspects via `A: B`, where `B` is draft enabled. - * Note that when an entity extends two other entities of which one has drafts enabled and - * one has not, then the one that is later in the list of mixins "wins": + * This includes thwo scenarios: + * - (a) Entities that are part of a service and have the annotation ＠odata.draft.enabled + * - (b) Entities that are draft enabled propagate this property down through compositions. + * NOTE: The compositions themselves must not be draft enabled, otherwise no draft entity will be generated for them * @param {any} csn - the entity * @example - * ```ts - * ＠odata.draft.enabled true - * entity T {} - * ＠odata.draft.enabled false - * entity F {} - * entity A: T,F {} // draft not enabled - * entity B: F,T {} // draft enabled - * ``` + * (a) + * ```cds + * // service.cds + * service MyService { + * ＠odata.draft.enabled true + * entity A {} * - * (b) Draft enabled projections make the entity we project on draft enabled. - * @example - * ```ts - * ＠odata.draft.enabled: true - * entity A as projection on B {} - * entity B {} // draft enabled + * ＠odata.draft.enabled true + * entity B {} + * } * ``` - * - * (c) Entities that are draft enabled propagate this property down through compositions: - * - * ```ts - * ＠odata.draft.enabled: true - * entity A { - * b: Composition of B + * @example + * (b) + * ```cds + * // service.cds + * service MyService { + * ＠odata.draft.enabled: true + * entity A { + * b: Composition of B + * } + * entity B {} // draft enabled * } - * entity B {} // draft enabled * ``` */ -function unrollDraftability(csn) { - new DraftUnroller().unroll(csn) +function collectDraftEnabledEntities(csn) { + new DraftEnabledEntityCollector().run(csn) } /** @@ -320,15 +285,6 @@ function propagateForeignKeys(csn) { } } -/** - * - * @param {any} csn - complete csn - */ -function amendCSN(csn) { - unrollDraftability(csn) - propagateForeignKeys(csn) -} - /** * @param {EntityCSN} entity - the entity */ @@ -349,7 +305,7 @@ const getProjectionAliases = entity => { } module.exports = { - amendCSN, + collectDraftEnabledEntities, isView, isProjection, isViewOrProjection, diff --git a/lib/visitor.js b/lib/visitor.js index 3ffde294..5b172af6 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -2,7 +2,7 @@ const util = require('./util') -const { amendCSN, isView, isUnresolved, propagateForeignKeys, isDraftEnabled, isType, isProjection, getMaxCardinality, isViewOrProjection, isEnum } = require('./csn') +const { isView, isUnresolved, propagateForeignKeys, collectDraftEnabledEntities, isDraftEnabled, isType, isProjection, getMaxCardinality, isViewOrProjection, isEnum } = require('./csn') // eslint-disable-next-line no-unused-vars const { SourceFile, FileRepository, Buffer, Path } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') @@ -40,8 +40,10 @@ class Visitor { * @param {{xtended: CSN, inferred: CSN}} csn - root CSN */ constructor(csn) { - amendCSN(csn.xtended) + propagateForeignKeys(csn.xtended) propagateForeignKeys(csn.inferred) + // has to be executed on the inferred model as autoexposed entities are not included in the xtended csn + collectDraftEnabledEntities(csn.inferred) this.csn = csn /** @type {Context[]} **/ @@ -283,16 +285,17 @@ class Visitor { // CLASS WITH ADDED ASPECTS file.addImport(baseDefinitions.path) docify(entity.doc).forEach(d => { buffer.add(d) }) - buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(clean, entity).join('\n')}}`) + buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(fq, clean, entity).join('\n')}}`) this.contexts.pop() } /** + * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity * @param {EntityCSN} entity - the entity to generate the static contents for */ - #staticClassContents(clean, entity) { - return isDraftEnabled(entity) ? [`static drafts: typeof ${clean}`] : [] + #staticClassContents(fq, clean, entity) { + return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}`] : [] } /** @@ -348,9 +351,6 @@ class Visitor { ? this.csn.inferred.definitions[fq] : entity - // draft enablement is stored in csn.xtended. Iff we took the entity from csn.inferred, we have to carry the draft-enablement over at this point - target['@odata.draft.enabled'] = isDraftEnabled(entity) - this.#aspectify(fq, target, buffer, { cleanName: singular }) buffer.add(overrideNameProperty(singular, entity.name)) @@ -366,7 +366,7 @@ class Visitor { } // plural can not be a type alias to $singular[] but needs to be a proper class instead, // so it can get passed as value to CQL functions. - const additionalProperties = this.#staticClassContents(singular, entity) + const additionalProperties = this.#staticClassContents(fq, singular, entity) additionalProperties.push('$count?: number') docify(entity.doc).forEach(d => { buffer.add(d) }) buffer.add(`export class ${plural} extends Array<${singular}> {${additionalProperties.join('\n')}}`) diff --git a/test/unit/draft.test.js b/test/unit/draft.test.js index 74c63216..ebe97651 100644 --- a/test/unit/draft.test.js +++ b/test/unit/draft.test.js @@ -1,7 +1,6 @@ 'use strict' const path = require('path') -const { describe, beforeAll, test, expect } = require('@jest/globals') const { ASTWrapper } = require('../ast') const { locations, prepareUnitTest } = require('../util') @@ -9,65 +8,31 @@ const draftable_ = (entity, ast) => ast.find(n => n.name === entity && n.members const draftable = (entity, ast, plural = e => `${e}_`) => draftable_(entity, ast) && draftable_(plural(entity), ast) describe('bookshop', () => { - test('Projections Up and Down', async () => { + test('Draft via root and compositions', async () => { const paths = (await prepareUnitTest('draft/catalog-service.cds', locations.testOutput('bookshop_projection'))).paths const service = new ASTWrapper(path.join(paths[1], 'index.ts')).tree const model = new ASTWrapper(path.join(paths[2], 'index.ts')).tree + // root and composition become draft enabled expect(draftable('Book', service, () => 'Books')).toBeTruthy() expect(draftable('Publisher', service, () => 'Publishers')).toBeTruthy() - expect(draftable('Book', model, () => 'Books')).toBeTruthy() - expect(draftable('Publisher', model, () => 'Publishers')).toBeTruthy() + + // associated entity will not become draft enabled + expect(draftable('Author', service, () => 'Authors')).toBeFalsy() + + // non-service entities will not be draft enabled + expect(draftable('Book', model, () => 'Books')).toBeFalsy() + expect(draftable('Publisher', model, () => 'Publishers')).toBeFalsy() + expect(draftable('Author', model, () => 'Authors')).toBeFalsy() }) -}) -describe('@odata.draft.enabled', () => { - let ast + test('Draft-enabled composition produces compiler error', async () => { + const spyOnConsole = jest.spyOn(console, 'error') + await prepareUnitTest('draft/catalog-service-error.cds', locations.testOutput('bookshop_projection'), {typerOptions: {logLevel: 'ERROR'}}) - beforeAll(async () => ast = (await prepareUnitTest('draft/model.cds', locations.testOutput('draft_test'))).astw.tree) - - test('Direct Annotation', async () => expect(draftable('A', ast)).toBeTruthy()) - - test('First Level Inheritance', async () => expect(draftable('B', ast)).toBeTruthy()) - - test('Explicit Override via Inheritance', async () => expect(draftable('C', ast)).not.toBeTruthy()) - - test('Inheritance of Explicit Override', async () => expect(draftable('D', ast)).not.toBeTruthy()) - - test('Declaration With true', async () => expect(draftable('E', ast)).toBeTruthy()) - - test('Multiple Inheritance With Most Significant true', async () => expect(draftable('F', ast)).toBeTruthy()) - - test('Multiple Inheritance With Most Significant false', async () => expect(draftable('G', ast)).not.toBeTruthy()) - - test('Draftable by Association/ Composition', async () => { - expect(draftable('H', ast)).not.toBeTruthy() - expect(draftable('I', ast)).not.toBeTruthy() - expect(draftable('J', ast)).not.toBeTruthy() - expect(draftable('K', ast)).not.toBeTruthy() - }) - - test('Unchanged by Association/ Composition', async () => { - expect(draftable('L', ast)).not.toBeTruthy() - expect(draftable('M', ast)).not.toBeTruthy() - }) - - test('Precedence Over Explicit Annotation', async () => { - expect(draftable('P', ast)).toBeTruthy() - expect(draftable('Q', ast)).toBeTruthy() - }) - - test('Via Projection', async () => expect(draftable('PA', ast)).toBeTruthy()) - - test('Transitive Via Projection and Composition', async () => { - expect(draftable('ProjectedReferrer', ast)).toBeTruthy() - expect(draftable('Referrer', ast)).toBeTruthy() - expect(draftable('Referenced', ast)).toBeTruthy() - }) - - test('Transitive Via Multiple Levels of Projection', async () => { - expect(draftable('Foo', ast)).toBeTruthy() - expect(draftable('ProjectedFoo', ast)).toBeTruthy() - expect(draftable('ProjectedProjectedFoo', ast)).toBeTruthy() + expect(spyOnConsole).toHaveBeenCalledWith( + '[cds-typer] -', + 'Composition in draft-enabled entity can\'t lead to another entity with "@odata.draft.enabled" (in entity: "bookshop.service.CatalogService.Books"/element: publishers)!' + ) }) }) \ No newline at end of file diff --git a/test/unit/files/draft/catalog-service-error.cds b/test/unit/files/draft/catalog-service-error.cds new file mode 100644 index 00000000..2be300e6 --- /dev/null +++ b/test/unit/files/draft/catalog-service-error.cds @@ -0,0 +1,10 @@ +namespace bookshop.service; + +using bookshop as my from './data-model'; + +service CatalogService { + @odata.draft.enabled + entity Books as projection on my.Books; + @odata.draft.enabled + entity Publishers as projection on my.Publishers; +} \ No newline at end of file diff --git a/test/unit/files/draft/catalog-service.cds b/test/unit/files/draft/catalog-service.cds index 75e3d7d2..59ab2157 100644 --- a/test/unit/files/draft/catalog-service.cds +++ b/test/unit/files/draft/catalog-service.cds @@ -5,5 +5,6 @@ using bookshop as my from './data-model'; service CatalogService { @odata.draft.enabled entity Books as projection on my.Books; - entity Publishers as projection on my.Publishers; + + entity Authors as projection on my.Authors; } \ No newline at end of file diff --git a/test/unit/files/draft/data-model.cds b/test/unit/files/draft/data-model.cds index d885452a..c2a9bd66 100644 --- a/test/unit/files/draft/data-model.cds +++ b/test/unit/files/draft/data-model.cds @@ -11,8 +11,13 @@ aspect managed { entity Books : managed, cuid { title : String; + author : Association to Authors; publishers : Composition of many Publishers - on publishers.book = $self; + on publishers.book = $self; +} + +entity Authors : managed, cuid { + name : String; } entity Publishers : managed, cuid { diff --git a/test/unit/files/draft/model.cds b/test/unit/files/draft/model.cds deleted file mode 100644 index b8768299..00000000 --- a/test/unit/files/draft/model.cds +++ /dev/null @@ -1,51 +0,0 @@ -namespace draft_test; - -@odata.draft.enabled -entity A {} -entity B: A {} -@odata.draft.enabled: false -entity C: B {} -entity D: C {} -@odata.draft.enabled: true -entity E: D {} -entity F: C,E {} -entity G: E,C {} - -// don't become draftable -entity H { ref: Association to A } -entity I { ref: Association to many A } -// should not become draftable -entity J { ref: Composition of A } -entity K { ref: Composition of many A } - -// should not -entity L { ref: Composition of C } -entity M { ref: Composition of many C } - -@odata.draft.enabled: true -entity N { ref: Composition of many O } -// ! should be enabled -entity O {} - -@odata.draft.enabled: true -entity P { ref: Association to C } -@odata.draft.enabled: false -entity Q { } -@odata.draft.enabled: true -entity R { ref: Composition of Q } - -entity PA as projection on A {} - -// propagate from projection to referenced entity -entity Referenced {} -entity Referrer { - ref: Composition of Referenced -} -@odata.draft.enabled: true -entity ProjectedReferrer as projection on Referrer {} - -// propagate over two levels of projection -entity Foo {} -entity ProjectedFoo as projection on Foo {} -@odata.draft.enabled: true -entity ProjectedProjectedFoo as projection on ProjectedFoo {} \ No newline at end of file diff --git a/test/util.js b/test/util.js index d0c863d0..adc66b91 100644 --- a/test/util.js +++ b/test/util.js @@ -183,7 +183,8 @@ async function prepareUnitTest(model, outputDirectory, parameters = {}) { await checkTranspilation(tsFiles) } configuration.setFrom(configurationBefore) - return { astw: new ASTWrapper(path.join(parameters.fileSelector(paths), 'index.ts')), paths } + // return undefined for astw in case type-generation failed + return { astw: paths?.length > 0 ? new ASTWrapper(path.join(parameters.fileSelector(paths), 'index.ts')) : undefined, paths } } From 0b5a7a35d998b23f10c16f8885e81d9475d6d88b Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 5 Oct 2024 17:45:05 +0200 Subject: [PATCH 2/9] refactor: rename cds file that causes transpilation errors --- test/unit/draft.test.js | 2 +- .../{catalog-service-error.cds => error-catalog-service.cds} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/unit/files/draft/{catalog-service-error.cds => error-catalog-service.cds} (100%) diff --git a/test/unit/draft.test.js b/test/unit/draft.test.js index ebe97651..e2565b41 100644 --- a/test/unit/draft.test.js +++ b/test/unit/draft.test.js @@ -28,7 +28,7 @@ describe('bookshop', () => { test('Draft-enabled composition produces compiler error', async () => { const spyOnConsole = jest.spyOn(console, 'error') - await prepareUnitTest('draft/catalog-service-error.cds', locations.testOutput('bookshop_projection'), {typerOptions: {logLevel: 'ERROR'}}) + await prepareUnitTest('draft/error-catalog-service.cds', locations.testOutput('bookshop_projection'), {typerOptions: {logLevel: 'ERROR'}}) expect(spyOnConsole).toHaveBeenCalledWith( '[cds-typer] -', diff --git a/test/unit/files/draft/catalog-service-error.cds b/test/unit/files/draft/error-catalog-service.cds similarity index 100% rename from test/unit/files/draft/catalog-service-error.cds rename to test/unit/files/draft/error-catalog-service.cds From 8c13983042e2911a2464b3efeb1b0da0fdb96fae Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 5 Oct 2024 18:07:26 +0200 Subject: [PATCH 3/9] fix: fixes lint issues --- eslint.config.js | 6 ++---- lib/csn.js | 2 +- lib/visitor.js | 4 ++-- test/unit/draft.test.js | 3 ++- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 0f74f434..11428c5c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -10,12 +10,10 @@ module.exports = [ ecmaVersion: 'latest', sourceType: 'commonjs', globals: { - ...require('globals').node + ...require('globals').node, + jest: true } }, - env: { - jest: true - }, files: ['**/*.js'], plugins: { '@stylistic/js': require('@stylistic/eslint-plugin-js') diff --git a/lib/csn.js b/lib/csn.js index 93c0f714..749b3dbe 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -194,7 +194,7 @@ class DraftEnabledEntityCollector { // note to self: following doc uses ＠ homoglyph instead of @, as the latter apparently has special semantics in code listings /** - * We collect all entities that are draft enabled. + * We collect all entities that are draft enabled. * (@see `@sap/cds-compiler/lib/transform/draft/db.js#generateDraft`) * * This includes thwo scenarios: diff --git a/lib/visitor.js b/lib/visitor.js index 5b172af6..e3ae3154 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -292,9 +292,9 @@ class Visitor { /** * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity - * @param {EntityCSN} entity - the entity to generate the static contents for + * @param {EntityCSN} _entity - the entity to generate the static contents for */ - #staticClassContents(fq, clean, entity) { + #staticClassContents(fq, clean, _entity) { return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}`] : [] } diff --git a/test/unit/draft.test.js b/test/unit/draft.test.js index e2565b41..bda6ebab 100644 --- a/test/unit/draft.test.js +++ b/test/unit/draft.test.js @@ -2,6 +2,7 @@ const path = require('path') const { ASTWrapper } = require('../ast') +const { describe, test, expect } = require('@jest/globals') const { locations, prepareUnitTest } = require('../util') const draftable_ = (entity, ast) => ast.find(n => n.name === entity && n.members.find(({name}) => name === 'drafts')) @@ -16,7 +17,7 @@ describe('bookshop', () => { // root and composition become draft enabled expect(draftable('Book', service, () => 'Books')).toBeTruthy() expect(draftable('Publisher', service, () => 'Publishers')).toBeTruthy() - + // associated entity will not become draft enabled expect(draftable('Author', service, () => 'Authors')).toBeFalsy() From 2d0d981a6ec6df6e25c6483fe2e2b2a3375acc1f Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Mon, 7 Oct 2024 17:45:47 +0200 Subject: [PATCH 4/9] feat: create dedicated classes/modules for draft --- lib/components/basedefs.js | 7 +++++++ lib/components/javascript.js | 4 +++- lib/file.js | 29 +++++++++++++++++++++++++++++ lib/visitor.js | 30 +++++++++++++++++++++++++++--- 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/lib/components/basedefs.js b/lib/components/basedefs.js index 763fcf3b..855fe336 100644 --- a/lib/components/basedefs.js +++ b/lib/components/basedefs.js @@ -38,6 +38,13 @@ export type EntitySet = T[] & { data (input:object) : T }; +export class DraftEntity extends Entity { + declare IsActiveEntity?: boolean | null + declare HasActiveEntity?: boolean | null + declare HasDraftEntity?: boolean | null + declare DraftAdministrativeData_DraftUUID?: string | null +} + export type DeepRequired = { [K in keyof T]: DeepRequired } & Exclude, null>; diff --git a/lib/components/javascript.js b/lib/components/javascript.js index a03c3c67..955fdf87 100644 --- a/lib/components/javascript.js +++ b/lib/components/javascript.js @@ -7,7 +7,9 @@ const proxyAccessFunction = function (fqParts, opts = {}) { return new Proxy(target, { get: function (target, prop) { if (cds.entities) { - target.__proto__ = cds.entities(fqParts[0])[fqParts[1]] + const entity = cds.entities(fqParts[0])[fqParts[1]] + // check fq for 'drafts', then delegate to drafts of entity + target.__proto__ = fqParts.length > 2 && fqParts[2] === 'drafts' ? entity.drafts : entity // overwrite/simplify getter after cds.entities is accessible this.get = (target, prop) => target[prop] return target[prop] diff --git a/lib/file.js b/lib/file.js index ba27b8eb..9430f33a 100644 --- a/lib/file.js +++ b/lib/file.js @@ -9,6 +9,7 @@ const { empty } = require('./components/typescript') const { proxyAccessFunction } = require('./components/javascript') const { createObjectOf } = require('./components/wrappers') const { configuration } = require('./config') +const { isDraftEnabled } = require('./csn') const AUTO_GEN_NOTE = '// This is an automatically generated file. Please do not change its contents manually!' @@ -475,6 +476,33 @@ class SourceFile extends File { } } } + /** + * Returns exports for draft entities or an empty array if the entity + * is not draft enabled + * @param {string} singular - singular name of entity + * @param {string} original - original name of entity + * @returns {string[]} exports with singular/plural of draft + */ + #getDraftEntityJsExports(singular, original) { + const namespace = this.path.asNamespace() + if (!isDraftEnabled(`${namespace}.${original}`)) return [] + + const comment = `// ${original}.drafts ` + + if (configuration.useEntitiesProxy) { + return [ + comment, + `module.exports.${singular}Draft = createEntityProxy(['${namespace}', '${original}', 'drafts'], { target: { is_singular: true } })`, + `module.exports.${singular}Drafts = createEntityProxy(['${namespace}', '${original}', 'drafts'])` + ] + } else { + return [ + comment, + `module.exports.${singular}Draft ={ is_singular: true, __proto__: csn.${original}.drafts }`, + `module.exports.${singular}Drafts = csn.${original}.drafts` + ] + } + } toJSExports() { return this.#getJSExportBoilerplate() // boilerplate .concat( @@ -510,6 +538,7 @@ class SourceFile extends File { const rhs = plural === original ? pluralRhs : singularRhs exports.push(`module.exports.${original} = ${rhs}`) } + exports.push(...this.#getDraftEntityJsExports(singular, original)) return exports }) ) // singular -> plural aliases diff --git a/lib/visitor.js b/lib/visitor.js index e3ae3154..e746317c 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -282,6 +282,9 @@ class Visitor { }, '};') // end of generated class }, '}') // end of aspect + // DRAFT CLASSES + this.#printDraftClasses(fq, buffer, identSingular(clean), identAspect(clean)) + // CLASS WITH ADDED ASPECTS file.addImport(baseDefinitions.path) docify(entity.doc).forEach(d => { buffer.add(d) }) @@ -289,13 +292,34 @@ class Visitor { this.contexts.pop() } + /** + * Prints draft classes for draft enabled + * @param {string} fq - FQN of entity + * @param {Buffer} buffer - buffer to write exported drafts classes to + * @param {string} singular - singular name of parent entity + * @param {string} aspectFunction - class creator function of parent entity + */ + #printDraftClasses(fq, buffer, singular, aspectFunction) { + if (!isDraftEnabled(fq)) return + const singularDraft = `${singular}Draft` + // add singular draft + buffer.addIndentedBlock(`export class ${singularDraft} extends ${aspectFunction}(__.DraftEntity) {`, () => { + buffer.add('static readonly kind: \'entity\' | \'type\' | \'aspect\' = \'entity\'') + this.#printStaticKeys(buffer, singularDraft) + }, '}') + + // add plural draft + buffer.add(`export class ${singular}Drafts extends Array<${singularDraft}> { $count?: number }`) + } + /** * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity * @param {EntityCSN} _entity - the entity to generate the static contents for + * @param {boolean} [isPlural] - `true` if passed entity is plural */ - #staticClassContents(fq, clean, _entity) { - return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}`] : [] + #staticClassContents(fq, clean, _entity, isPlural = false) { + return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}Draft${isPlural ? 's' : ''}`] : [] } /** @@ -366,7 +390,7 @@ class Visitor { } // plural can not be a type alias to $singular[] but needs to be a proper class instead, // so it can get passed as value to CQL functions. - const additionalProperties = this.#staticClassContents(fq, singular, entity) + const additionalProperties = this.#staticClassContents(fq, singular, entity, true) additionalProperties.push('$count?: number') docify(entity.doc).forEach(d => { buffer.add(d) }) buffer.add(`export class ${plural} extends Array<${singular}> {${additionalProperties.join('\n')}}`) From 4346eabc542e0f2b1cf5dc060920a1a935ef2505 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Mon, 21 Oct 2024 22:42:13 +0200 Subject: [PATCH 5/9] refactor: replace classes with generic types for `.drafts` properties --- lib/components/basedefs.js | 13 ++++++++----- lib/visitor.js | 34 ++++++---------------------------- 2 files changed, 14 insertions(+), 33 deletions(-) diff --git a/lib/components/basedefs.js b/lib/components/basedefs.js index 855fe336..bf3758d1 100644 --- a/lib/components/basedefs.js +++ b/lib/components/basedefs.js @@ -38,13 +38,16 @@ export type EntitySet = T[] & { data (input:object) : T }; -export class DraftEntity extends Entity { - declare IsActiveEntity?: boolean | null - declare HasActiveEntity?: boolean | null - declare HasDraftEntity?: boolean | null - declare DraftAdministrativeData_DraftUUID?: string | null +export type DraftEntity = T & { + IsActiveEntity?: boolean | null + HasActiveEntity?: boolean | null + HasDraftEntity?: boolean | null + DraftAdministrativeData_DraftUUID?: string | null } +export type DraftOf = { new(...args: any[]): DraftEntity } +export type DraftsOf = typeof Array> + export type DeepRequired = { [K in keyof T]: DeepRequired } & Exclude, null>; diff --git a/lib/visitor.js b/lib/visitor.js index e746317c..44adce33 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -282,44 +282,22 @@ class Visitor { }, '};') // end of generated class }, '}') // end of aspect - // DRAFT CLASSES - this.#printDraftClasses(fq, buffer, identSingular(clean), identAspect(clean)) - // CLASS WITH ADDED ASPECTS file.addImport(baseDefinitions.path) docify(entity.doc).forEach(d => { buffer.add(d) }) - buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(fq, clean, entity).join('\n')}}`) + buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(fq, clean).join('\n')}}`) this.contexts.pop() } - /** - * Prints draft classes for draft enabled - * @param {string} fq - FQN of entity - * @param {Buffer} buffer - buffer to write exported drafts classes to - * @param {string} singular - singular name of parent entity - * @param {string} aspectFunction - class creator function of parent entity - */ - #printDraftClasses(fq, buffer, singular, aspectFunction) { - if (!isDraftEnabled(fq)) return - const singularDraft = `${singular}Draft` - // add singular draft - buffer.addIndentedBlock(`export class ${singularDraft} extends ${aspectFunction}(__.DraftEntity) {`, () => { - buffer.add('static readonly kind: \'entity\' | \'type\' | \'aspect\' = \'entity\'') - this.#printStaticKeys(buffer, singularDraft) - }, '}') - - // add plural draft - buffer.add(`export class ${singular}Drafts extends Array<${singularDraft}> { $count?: number }`) - } - /** * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity - * @param {EntityCSN} _entity - the entity to generate the static contents for * @param {boolean} [isPlural] - `true` if passed entity is plural */ - #staticClassContents(fq, clean, _entity, isPlural = false) { - return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}Draft${isPlural ? 's' : ''}`] : [] + #staticClassContents(fq, clean, isPlural = false) { + if (!isDraftEnabled(fq)) return [] + const draftsType = isPlural ? '__.DraftsOf' : '__.DraftOf' + return [`static drafts: ${draftsType}<${clean}>`] } /** @@ -390,7 +368,7 @@ class Visitor { } // plural can not be a type alias to $singular[] but needs to be a proper class instead, // so it can get passed as value to CQL functions. - const additionalProperties = this.#staticClassContents(fq, singular, entity, true) + const additionalProperties = this.#staticClassContents(fq, singular, true) additionalProperties.push('$count?: number') docify(entity.doc).forEach(d => { buffer.add(d) }) buffer.add(`export class ${plural} extends Array<${singular}> {${additionalProperties.join('\n')}}`) From b227ccc942897001043d161ceb9a357f25fc4a3e Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Mon, 21 Oct 2024 22:42:58 +0200 Subject: [PATCH 6/9] test: adding compile tests for new draft entity fields --- test/unit/files/draft/model.cds | 1 + test/unit/files/draft/model.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 test/unit/files/draft/model.cds create mode 100644 test/unit/files/draft/model.ts diff --git a/test/unit/files/draft/model.cds b/test/unit/files/draft/model.cds new file mode 100644 index 00000000..fcdd2fb0 --- /dev/null +++ b/test/unit/files/draft/model.cds @@ -0,0 +1 @@ +using from './catalog-service'; \ No newline at end of file diff --git a/test/unit/files/draft/model.ts b/test/unit/files/draft/model.ts new file mode 100644 index 00000000..84e64cce --- /dev/null +++ b/test/unit/files/draft/model.ts @@ -0,0 +1,31 @@ +import cds from '@sap/cds' +import {DraftEntity} from '#cds-models/_' +import {Books, Publishers} from '#cds-models/bookshop/service/CatalogService' + +export class CatalogService extends cds.ApplicationService { + async init() { + + this.after("READ", Publishers.drafts, publishers => { + if (!publishers?.length) return + publishers.forEach(p => { + p.name + p.HasActiveEntity + p.HasDraftEntity + p.IsActiveEntity + p.DraftAdministrativeData_DraftUUID + }) + }) + + this.after("READ", Books.drafts, books => { + if (!books?.length) return + books.forEach(b => { + b.title + b.HasActiveEntity + b.HasDraftEntity + b.IsActiveEntity + b.DraftAdministrativeData_DraftUUID + }) + }) + return super.init() + } +} \ No newline at end of file From 8917e89578052b5c4d75486bd0215972439066ec Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Wed, 23 Oct 2024 15:16:57 +0200 Subject: [PATCH 7/9] refactor: remove draft exports from index.js files --- lib/file.js | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/lib/file.js b/lib/file.js index dbf75fc5..c805486d 100644 --- a/lib/file.js +++ b/lib/file.js @@ -9,7 +9,6 @@ const { empty } = require('./components/typescript') const { proxyAccessFunction } = require('./components/javascript') const { createObjectOf } = require('./components/wrappers') const { configuration } = require('./config') -const { isDraftEnabled } = require('./csn') const AUTO_GEN_NOTE = '// This is an automatically generated file. Please do not change its contents manually!' @@ -481,33 +480,6 @@ class SourceFile extends File { } } } - /** - * Returns exports for draft entities or an empty array if the entity - * is not draft enabled - * @param {string} singular - singular name of entity - * @param {string} original - original name of entity - * @returns {string[]} exports with singular/plural of draft - */ - #getDraftEntityJsExports(singular, original) { - const namespace = this.path.asNamespace() - if (!isDraftEnabled(`${namespace}.${original}`)) return [] - - const comment = `// ${original}.drafts ` - - if (configuration.useEntitiesProxy) { - return [ - comment, - `module.exports.${singular}Draft = createEntityProxy(['${namespace}', '${original}', 'drafts'], { target: { is_singular: true } })`, - `module.exports.${singular}Drafts = createEntityProxy(['${namespace}', '${original}', 'drafts'])` - ] - } else { - return [ - comment, - `module.exports.${singular}Draft ={ is_singular: true, __proto__: csn.${original}.drafts }`, - `module.exports.${singular}Drafts = csn.${original}.drafts` - ] - } - } toJSExports() { return this.#getJSExportBoilerplate() // boilerplate .concat( @@ -543,7 +515,6 @@ class SourceFile extends File { const rhs = plural === original ? pluralRhs : singularRhs exports.push(`module.exports.${original} = ${rhs}`) } - exports.push(...this.#getDraftEntityJsExports(singular, original)) return exports }) ) // singular -> plural aliases From e101d118012e70e75c86fb8b3f5043998f1c2f27 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Wed, 23 Oct 2024 15:18:00 +0200 Subject: [PATCH 8/9] refactor: remove draft exports from index.js files --- lib/components/javascript.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/components/javascript.js b/lib/components/javascript.js index 955fdf87..a03c3c67 100644 --- a/lib/components/javascript.js +++ b/lib/components/javascript.js @@ -7,9 +7,7 @@ const proxyAccessFunction = function (fqParts, opts = {}) { return new Proxy(target, { get: function (target, prop) { if (cds.entities) { - const entity = cds.entities(fqParts[0])[fqParts[1]] - // check fq for 'drafts', then delegate to drafts of entity - target.__proto__ = fqParts.length > 2 && fqParts[2] === 'drafts' ? entity.drafts : entity + target.__proto__ = cds.entities(fqParts[0])[fqParts[1]] // overwrite/simplify getter after cds.entities is accessible this.get = (target, prop) => target[prop] return target[prop] From e73a0e071c2f11c198484253245b164ebfababf0 Mon Sep 17 00:00:00 2001 From: stockbal Date: Thu, 24 Oct 2024 11:25:25 +0200 Subject: [PATCH 9/9] refactor: add wrappers for draft types --- lib/components/wrappers.js | 16 ++++++++++++++++ lib/visitor.js | 5 ++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/components/wrappers.js b/lib/components/wrappers.js index b6816893..6f4884e0 100644 --- a/lib/components/wrappers.js +++ b/lib/components/wrappers.js @@ -17,6 +17,20 @@ const createKey = t => `${base}.Key<${t}>` */ const createKeysOf = t => `${base}.KeysOf<${t}>` +/** + * Wraps type into DraftOf type. + * @param {string} t - the type name. + * @returns {string} + */ +const createDraftOf = t => `${base}.DraftOf<${t}>` + +/** + * Wraps type into DraftsOf type. + * @param {string} t - the type name. + * @returns {string} + */ +const createDraftsOf = t => `${base}.DraftsOf<${t}>` + /** * Wraps type into ElementsOf type. * @param {string} t - the type name. @@ -114,6 +128,8 @@ const stringIdent = s => `'${s}'` module.exports = { createArrayOf, + createDraftOf, + createDraftsOf, createKey, createKeysOf, createElementsOf, diff --git a/lib/visitor.js b/lib/visitor.js index 7bf69bf4..b82a2dec 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -8,7 +8,7 @@ const { SourceFile, FileRepository, Buffer, Path } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') const { Resolver } = require('./resolution/resolver') const { LOG } = require('./logging') -const { docify, createPromiseOf, createUnionOf, createKeysOf, createElementsOf, stringIdent } = require('./components/wrappers') +const { docify, createPromiseOf, createUnionOf, createKeysOf, createElementsOf, stringIdent, createDraftsOf, createDraftOf } = require('./components/wrappers') const { csnToEnumPairs, propertyToInlineEnumName, isInlineEnumType, stringifyEnumType } = require('./components/enum') const { isReferenceType } = require('./components/reference') const { empty } = require('./components/typescript') @@ -340,8 +340,7 @@ class Visitor { */ #staticClassContents(fq, clean, isPlural = false) { if (!isDraftEnabled(fq)) return [] - const draftsType = isPlural ? '__.DraftsOf' : '__.DraftOf' - return [`static drafts: ${draftsType}<${clean}>`] + return [`static drafts: ${isPlural ? createDraftsOf(clean) : createDraftOf(clean)}`] } /**