diff --git a/docs/site/Importing-LB3-models.md b/docs/site/Importing-LB3-models.md index 9284db890f1e..9ef79f264cab 100644 --- a/docs/site/Importing-LB3-models.md +++ b/docs/site/Importing-LB3-models.md @@ -60,24 +60,6 @@ _The tracking GitHub issue: Workaround: define relational metadata & navigational properties manually. -### Models inheriting from custom base class - -_The tracking GitHub issue: -[loopback-next#3813](https://github.com/strongloop/loopback-next/issues/3813)_ - -Models inheriting from application-specific models (including LB3 built-in -models like `User`) cannot be imported yet. - -Workaround: - -1. Modify your LB3 model to inherit from `Model`, `PersistedModel` or - `KeyValueModel`. - -2. Import the model to LB4 - -3. Update the imported model to inherit for the desired application-specific - model. - ### MongoDB's `ObjectID` type The tracking GitHub issue: diff --git a/packages/cli/generators/import-lb3-models/index.js b/packages/cli/generators/import-lb3-models/index.js index 5343e222064b..4808ba8091d1 100644 --- a/packages/cli/generators/import-lb3-models/index.js +++ b/packages/cli/generators/import-lb3-models/index.js @@ -48,6 +48,10 @@ module.exports = class Lb3ModelImporter extends BaseGenerator { this.destinationPath(), this.artifactInfo.outDir, ); + this.artifactInfo.modelDir = path.resolve( + this.artifactInfo.rootDir, + utils.modelsDir, + ); return super.setOptions(); } @@ -99,6 +103,14 @@ Learn more at https://loopback.io/doc/en/lb4/Importing-LB3-models.html this.modelNames = answers.modelNames; } + async loadExistingLb4Models() { + debug(`model list dir ${this.artifactInfo.modelDir}`); + this.existingLb4ModelNames = await utils.getArtifactList( + this.artifactInfo.modelDir, + 'model', + ); + } + async migrateSelectedModels() { if (this.shouldExit()) return; this.modelFiles = []; @@ -136,6 +148,20 @@ Learn more at https://loopback.io/doc/en/lb4/Importing-LB3-models.html ); debug('LB4 model data', templateData); + if (!templateData.isModelBaseBuiltin) { + const baseName = templateData.modelBaseClass; + if ( + !this.existingLb4ModelNames.includes(baseName) && + !this.modelNames.includes(baseName) + ) { + this.log( + 'Adding %s (base of %s) to the list of imported models.', + chalk.yellow(baseName), + chalk.yellow(name), + ); + this.modelNames.push(baseName); + } + } const fileName = utils.getModelFileName(name); const fullTargetPath = path.resolve(this.artifactInfo.relPath, fileName); debug('Model %s output file', name, fullTargetPath); diff --git a/packages/cli/generators/import-lb3-models/migrate-model.js b/packages/cli/generators/import-lb3-models/migrate-model.js index 4e5aca904fe1..22e7f8a3f770 100644 --- a/packages/cli/generators/import-lb3-models/migrate-model.js +++ b/packages/cli/generators/import-lb3-models/migrate-model.js @@ -17,6 +17,7 @@ const { findBuiltinType, } = require('../model/property-definition'); const chalk = require('chalk'); +const {isDeepStrictEqual} = require('util'); module.exports = { importLb3ModelDefinition, @@ -43,12 +44,29 @@ function importLb3ModelDefinition(modelCtor, log) { logNamingIssues(modelName, log); + const baseDefinition = modelCtor.base.definition; + const baseProps = {...baseDefinition.properties}; + + // Core LB3 models like PersistedModel come with an id property that's + // injected by juggler. We don't want to inherit that property, because + // in LB4, we want models to define the id property explicitly. + if (isCoreModel(modelCtor.base)) { + delete baseProps.id; + } + const templateData = { name: modelName, className: pascalCase(modelName), ...migrateBaseClass(modelCtor.settings.base), - properties: migrateModelProperties(modelCtor.definition.properties), - settings: migrateModelSettings(modelCtor.definition.settings, log), + properties: migrateModelProperties( + modelCtor.definition.properties, + baseProps, + ), + settings: migrateModelSettings( + modelCtor.definition.settings, + baseDefinition.settings, + log, + ), }; const settings = templateData.settings; @@ -59,7 +77,7 @@ function importLb3ModelDefinition(modelCtor, log) { return templateData; } -function migrateModelProperties(properties) { +function migrateModelProperties(properties = {}, inherited = {}) { const templateData = {}; // In LB 3.x, primary keys are typically contributed by connectors later in @@ -72,7 +90,16 @@ function migrateModelProperties(properties) { }); for (const prop in properties) { - const def = migratePropertyDefinition(properties[prop]); + const propDef = properties[prop]; + + // Skip the property if it was inherited from the base model (the parent) + const baseProp = inherited[prop]; + if (baseProp && isDeepStrictEqual(propDef, baseProp)) { + delete templateData[prop]; + continue; + } + + const def = migratePropertyDefinition(propDef); templateData[prop] = createPropertyTemplateData(def); } @@ -132,14 +159,13 @@ function migrateBaseClass(base) { }; } - // TODO: handle inheritance from application models - throw new Error( - 'Models inheriting from app-specific models cannot be migrated yet. ' + - `Base model configured: ${baseModelName}`, - ); + return { + modelBaseClass: baseModelName, + isModelBaseBuiltin: false, + }; } -function migrateModelSettings(settings = {}, log) { +function migrateModelSettings(settings = {}, inherited = {}, log) { // Shallow-clone the object to prevent modification of external data settings = {...settings}; @@ -147,6 +173,16 @@ function migrateModelSettings(settings = {}, log) { // LB3 models allow additional properties by default settings.strict = settings.strict === true; + // Remove settings inherited from the base model + for (const key in inherited) { + // Always emit the value of 'strict' setting, make it explicit + if (key === 'strict') continue; + + if (isDeepStrictEqual(settings[key], inherited[key])) { + delete settings[key]; + } + } + if (settings.forceId === 'auto') { // The value 'auto' is used when a parent model wants to let the child // model make the decision automatically, depending on whether the child @@ -204,3 +240,10 @@ function migrateModelSettings(settings = {}, log) { return settings; } + +function isCoreModel(modelCtor) { + const name = modelCtor.modelName; + return ( + name === 'Model' || name === 'PersistedModel' || name === 'KeyValueModel' + ); +} diff --git a/packages/cli/package.json b/packages/cli/package.json index e7d2e945c2b6..7812caf88bb6 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -28,6 +28,7 @@ "@types/ejs": "^3.0.1", "@types/node": "^10.17.16", "glob": "^7.1.6", + "loopback": "^3.26.0", "loopback-datasource-juggler": "^4.18.1", "mem-fs": "^1.1.3", "mem-fs-editor": "^6.0.0", @@ -50,7 +51,6 @@ "json5": "^2.1.1", "latest-version": "^5.1.0", "lodash": "^4.17.15", - "loopback": "^3.26.0", "minimist": "^1.2.0", "mkdirp": "^1.0.3", "natural-compare": "^1.4.0", diff --git a/packages/cli/snapshots/integration/generators/import-lb3-models.integration.snapshots.js b/packages/cli/snapshots/integration/generators/import-lb3-models.integration.snapshots.js index 7b852937c5f2..ac9c1f8ef17a 100644 --- a/packages/cli/snapshots/integration/generators/import-lb3-models.integration.snapshots.js +++ b/packages/cli/snapshots/integration/generators/import-lb3-models.integration.snapshots.js @@ -11,13 +11,7 @@ exports[`lb4 import-lb3-models imports CoffeeShop model from lb3-example app 1`] import {Entity, model, property} from '@loopback/repository'; @model({ - settings: { - strict: false, - forceId: false, - replaceOnPUT: true, - validateUpsert: true, - idInjection: true - } + settings: {strict: false, forceId: false, validateUpsert: true, idInjection: true} }) export class CoffeeShop extends Entity { @property({ @@ -62,3 +56,140 @@ exports[`lb4 import-lb3-models imports CoffeeShop model from lb3-example app 2`] export * from './coffee-shop.model'; `; + + +exports[`lb4 import-lb3-models imports a model inheriting from a custom base class 1`] = ` +import {model, property} from '@loopback/repository'; +import {UserBase} from '.'; + +@model({settings: {strict: false, customCustomerSetting: true}}) +export class Customer extends UserBase { + @property({ + type: 'boolean', + }) + isPreferred?: boolean; + + // Define well-known properties here + + // Indexer property to allow additional data + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [prop: string]: any; + + constructor(data?: Partial) { + super(data); + } +} + +export interface CustomerRelations { + // describe navigational properties here +} + +export type CustomerWithRelations = Customer & CustomerRelations; + +`; + + +exports[`lb4 import-lb3-models imports a model inheriting from a custom base class 2`] = ` +import {model, property} from '@loopback/repository'; +import {User} from '.'; + +@model({settings: {strict: false, customUserBaseSetting: true}}) +export class UserBase extends User { + @property({ + type: 'boolean', + required: true, + default: false, + }) + isAccountVerified: boolean; + + // Define well-known properties here + + // Indexer property to allow additional data + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [prop: string]: any; + + constructor(data?: Partial) { + super(data); + } +} + +export interface UserBaseRelations { + // describe navigational properties here +} + +export type UserBaseWithRelations = UserBase & UserBaseRelations; + +`; + + +exports[`lb4 import-lb3-models imports a model inheriting from a custom base class 3`] = ` +import {Entity, model, property} from '@loopback/repository'; + +@model({ + settings: { + strict: false, + caseSensitiveEmail: true, + hidden: ['password', 'verificationToken'], + maxTTL: 31556926, + ttl: 1209600 + } +}) +export class User extends Entity { + @property({ + type: 'number', + id: 1, + generated: true, + updateOnly: true, + }) + id?: number; + + @property({ + type: 'string', + }) + realm?: string; + + @property({ + type: 'string', + }) + username?: string; + + @property({ + type: 'string', + required: true, + }) + password: string; + + @property({ + type: 'string', + required: true, + }) + email: string; + + @property({ + type: 'boolean', + }) + emailVerified?: boolean; + + @property({ + type: 'string', + }) + verificationToken?: string; + + // Define well-known properties here + + // Indexer property to allow additional data + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [prop: string]: any; + + constructor(data?: Partial) { + super(data); + } +} + +export interface UserRelations { + // describe navigational properties here +} + +export type UserWithRelations = User & UserRelations; + +`; diff --git a/packages/cli/test/fixtures/import-lb3-models/app-using-model-inheritance.js b/packages/cli/test/fixtures/import-lb3-models/app-using-model-inheritance.js new file mode 100644 index 000000000000..0c0daa5be4dc --- /dev/null +++ b/packages/cli/test/fixtures/import-lb3-models/app-using-model-inheritance.js @@ -0,0 +1,44 @@ +// Copyright IBM Corp. 2020. All Rights Reserved. +// Node module: @loopback/cli +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +'use strict'; + +// In this file, we provide a LoopBack 3 application with a `Customer` model +// inheriting from a custom `UserBase` model that's inheriting from the LB3 +// built-in model `User`. + +const loopback = require('loopback'); +const app = loopback(); +module.exports = app; + +app.dataSource('db', {connector: 'memory'}); + +app.registry.createModel( + 'UserBase', + { + isAccountVerified: { + type: 'boolean', + required: true, + default: false, + }, + }, + { + base: 'User', + customUserBaseSetting: true, + }, +); +// Note that `UserBase` is not attached to any datasource! + +const Customer = app.registry.createModel( + 'Customer', + { + isPreferred: 'boolean', + }, + { + base: 'UserBase', + customCustomerSetting: true, + }, +); +app.model(Customer, {dataSource: 'db'}); diff --git a/packages/cli/test/integration/generators/import-lb3-models.integration.js b/packages/cli/test/integration/generators/import-lb3-models.integration.js index a468c5b689c7..c1ac32e74153 100644 --- a/packages/cli/test/integration/generators/import-lb3-models.integration.js +++ b/packages/cli/test/integration/generators/import-lb3-models.integration.js @@ -29,6 +29,10 @@ const COFFEE_SHOP_EXAMPLE = require.resolve( '../../../../../examples/lb3-application/lb3app/server/server', ); +const APP_USING_MODEL_INHERITANCE = require.resolve( + '../../fixtures/import-lb3-models/app-using-model-inheritance', +); + describe('lb4 import-lb3-models', function() { require('../lib/base-generator')(generator, {args: ['path-to-lb3-app']})(); @@ -91,4 +95,26 @@ describe('lb4 import-lb3-models', function() { 'AccessToken, User, RoleMapping, Role, ACL, Scope, CoffeeShop.', ); }); + + it('imports a model inheriting from a custom base class', async () => { + const outDir = path.join(SANDBOX_PATH, 'models-with-inheritance'); + + await testUtils + .executeGenerator(generator) + .inDir(SANDBOX_PATH, () => testUtils.givenLBProject(SANDBOX_PATH)) + .withArguments(APP_USING_MODEL_INHERITANCE) + .withPrompts({ + modelNames: ['Customer'], + }) + .withOptions({outDir}); + + // Verify the source code generated for the selected model + expectFileToMatchSnapshot(`${outDir}/customer.model.ts`); + + // Verify the source code generated for the custom base model + expectFileToMatchSnapshot(`${outDir}/user-base.model.ts`); + + // Verify the source code generated for the imported LB3 user model + expectFileToMatchSnapshot(`${outDir}/user.model.ts`); + }); }); diff --git a/packages/cli/test/unit/import-lb3-model/migrate-model.test.js b/packages/cli/test/unit/import-lb3-model/migrate-model.test.js index 12400d5f37f1..e0b6d1ca3e3a 100644 --- a/packages/cli/test/unit/import-lb3-model/migrate-model.test.js +++ b/packages/cli/test/unit/import-lb3-model/migrate-model.test.js @@ -33,7 +33,6 @@ describe('importLb3ModelDefinition', () => { expect(toJSON(modelData.settings)).to.deepEqual({ // By default, LB3 models are not strict strict: false, - replaceOnPUT: true, }); }); @@ -220,13 +219,33 @@ describe('importLb3ModelDefinition', () => { }); }); - it('refuses to import model extending a non-LB4-built-in (for now)', () => { - // Note: User is a built-in model in LB3 - const MyModel = givenLb3Model('MyModel', {}, {base: 'User'}); - expect(() => importLb3ModelDefinition(MyModel, log)).to.throw( - 'Models inheriting from app-specific models cannot be migrated yet. ' + - 'Base model configured: User', - ); + it('supports LB3 built-in base classes not available in LB4', () => { + const Customer = givenLb3Model('Customer', {}, {base: 'User'}); + const modelData = importLb3ModelDefinition(Customer, log); + + const options = getModelTemplateOptions(modelData); + expect(options).to.containDeep({ + modelBaseClass: 'User', + isModelBaseBuiltin: false, + }); + + log.resetHistory(); // ignore messages about ACLs & Relations + }); + + it('supports custom base classes', () => { + const {model: Admin} = givenLb3BaseAndChildModels({ + model: {name: 'Admin'}, + base: {name: 'MyUserBase'}, + }); + const modelData = importLb3ModelDefinition(Admin, log); + + const options = getModelTemplateOptions(modelData); + expect(options).to.containDeep({ + modelBaseClass: 'MyUserBase', + isModelBaseBuiltin: false, + }); + + log.resetHistory(); // disable assertion in afterEach hook }); }); @@ -309,6 +328,43 @@ describe('importLb3ModelDefinition', () => { }); }); + context('inheritance', () => { + let Customer, modelData; + beforeEach(function defineCustomerInheritingFromUser() { + Customer = givenLb3Model( + 'Customer', + { + isPreferred: {type: 'boolean', required: true}, + }, + { + base: 'User', + customCustomerSetting: true, + }, + ); + modelData = importLb3ModelDefinition(Customer, log); + log.resetHistory(); // ignore messages about ACLs & Relations + }); + + it('emits properties defined by the child model but not inherited', () => { + expect(Object.keys(modelData.properties)).to.deepEqual(['isPreferred']); + expect(modelData.properties) + .to.have.property('isPreferred') + .deepEqual({ + type: `'boolean'`, + tsType: 'boolean', + required: true, + }); + }); + + it('emits model-level settings defined by the model but not inherited', () => { + expect(toJSON(modelData.settings)).to.deepEqual({ + // By default, LB3 models are not strict + strict: false, + customCustomerSetting: true, + }); + }); + }); + context('relations', () => { it('reports a warning', () => { const app = givenLb3App(); @@ -432,6 +488,26 @@ function givenLb3Model( return ModelCtor; } +function givenLb3BaseAndChildModels({ + model, + base, + dataSourceConfig = {connector: 'memory'}, +}) { + const app = givenLb3App(); + app.dataSource('ds', dataSourceConfig); + const BaseCtor = app.registry.createModel( + base.name, + base.properties, + base.settings, + ); + const ModelCtor = app.registry.createModel(model.name, model.properties, { + base: base.name, + ...model.settings, + }); + app.model(ModelCtor, {dataSource: 'ds'}); + return {model: ModelCtor, base: BaseCtor}; +} + function getModelTemplateOptions(templateData) { const options = {...templateData};