Loopback-next: [CLI] Prevent usage of already used keywords

[Yaty]

Description / Steps to reproduce / Feature proposal

When using the CLI I can create a model named Entity, or Model. The model is generated but no warning will be emitted (this keyword is already used by Loopback).

Current Behavior

The model/controller/datasource/... is generated.

import {Model, model, property} from '@loopback/repository';

@model()
export class Model extends Model {
  constructor(data?: Partial<Model>) {
    super(data);
  }
}

Expected Behavior

Multiple options here:

1. add warnings in the creation process
2. prevent the component (model/controller/...) from being created
   
   
   
   1. by retrying
   
   
   2. by exiting
   
   
3. modify the generated file , for a model e.g:

import * as repository from '@loopback/repository';

@repository.model()
export class Model extends repository.Model {
  constructor(data?: Partial<Model>) {
    super(data);
  }
}

Keywords to consider

For controllers: nothing (overwriting existing controllers is already handled)

For datasources: nothing (overwriting existing datasources is already handled)

For models:

• Entity
• Model

For repositories:

• Repository

For services: ?

For openapi: nothing

Anything else ?

Acceptance criteria

Add a Yeoman prompt validator to reject model/repository/controller names that are the same as one of the built-in ones. This way, when a user enters invalid name, Yeoman will print an error and repeat the prompt.

• [ ] Enhance ArtifactGenerator.promptArtifactName() to allow subclasses (specific generators) to further customize the validation rules for artifact name. For example, we can introduce a protected property that subclasses can override.

  https://github.com/strongloop/loopback-next/blob/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/lib/artifact-generator.js#L60

For each of the following generators, provide validation rule that will reject problematic names:

• [ ] [lb4 controller](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/controller/index.js)
• [ ] [lb4 datasource](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/datasource/index.js)
• [ ] [lb4 interceptor](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/interceptor/index.js)
• [ ] [lb4 model](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/model/index.js)
• [ ] [lb4 observer](https://github.com/strongloop/loopback-next/blob/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/observer/index.js)
• [ ] [lb4 relation](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/relation/index.js)
• [ ] [lb4 repository](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/repository/index.js)
• [ ] [lb4 service](https://github.com/strongloop/loopback-next/tree/752db84f3e6f4984c35c2fc65f54eacae23e002e/packages/cli/generators/service/index.js)

---

🎆 Hacktoberfest 2020

Greetings :wave: to all Hacktoberfest 2020 participants!

Here are few tips 👀 to make your start easier, see also #6456:

• Before you start working on this issue, please leave a comment to let others know.
• This issue consists of several tasks to work on, it may feel like a too big effort to undertake. Don't worry! It's perfectly fine to pick just one item from the list and leave the rest for somebody else. In fact, we prefer to have a dedicated pull request for each part, to make it easier for us to review the changes and get the pull request landed faster. Baby steps FTW! Remember, every little helps.
• If you are new to GitHub pull requests, then you can learn about the process in Submitting a pull request to LoopBack 4.
• If this is your first contribution to LoopBack, then please take a look at our Developer guide
• Feel free to ask for help in #loopback-contributors channel, you can join our Slack workspace here.

[bajtos]

Personally, I would prefer the following option:

• prevent the component (model/controller/...) from being created by retrying

Ideally, we should check not only for built-in names like Model/Entity, but also for entities already defined in the project.

@Yaty would you like to contribute these improvements?

[Yaty]

I'm on it !
This option looks good to me.

[erogleva]

I'd like to work on this

[bajtos]

@erogleva awesome, let us know if you need any help to get started!

[erogleva]

It would be good for me to summarize the keywords which should be rejected for the different generators

For lb4 model -This seems clear and I've started implementing it (reject the names Model and Entity as well as all names of models already defined in the project)

For lb4 repository - Repository? But If you actually specify this keyword the generator will create a RepositoryRepository class which is weird but technically not an issue :smile: It would be an issue though if something like DefaultCrud is entered :smile:

For lb4 observer - LifeCycle.

For lb4 relation - It seems that creating a relation with the same foreign key name as a field which is already defined on the model removes the old definition and leaves only the new relation. If this is the desired behavior, perhaps I could add a hint that the current field will be overridden.

For lb4 controller, lb4 service and lb4 interceptor - Overriding existing controllers and services is already handled (as pointed out above). So are there any other problematic names here?

[bajtos]

Thank you @erogleva for the summary. I don't have detailed-enough knowledge, but your conclusions looks reasonable to me 👍🏻

For lb4 repository - Repository? But If you actually specify this keyword the generator will create a RepositoryRepository class which is weird but technically not an issue 😄 It would be an issue though if something like DefaultCrud is entered 😄

I think that means we should reject DefaultCrud as a user-provided name. Probably also KeyValue?

For lb4 relation - It seems that creating a relation with the same foreign key name as a field which is already defined on the model removes the old definition and leaves only the new relation. If this is the desired behavior, perhaps I could add a hint that the current field will be overridden.

@agnes512 I think you are most knowledgeable about relations, what's your opinion on the desired behavior of lb4 relation?

For lb4 controller, lb4 service and lb4 interceptor - Overriding existing controllers and services is already handled (as pointed out above). So are there any other problematic names here?

To be honest, I don't know. It's possible there are no "reserved" names to avoid. Maybe we can ignore these three CLI commands for now, and wait to see if there are any bug reports coming from our users?

[agnes512]

@erogleva thanks for bring up the issue!
I think for current lb4 relation, duplicate foreign keys or relation names would be rejected. Could you provide more details about the old definition getting removed? Cause it looks a bug to me. Thanks!

[erogleva]

I meant a case such as the following:

For example, if we have a model named Company with "address" as a string property

@model()
export class Company extends Entity {
  @property({
    type: 'string',
    required: true,
  })
  address: string;

  @property({
    type: 'string',
    id: true,
    generated: true,
  })
  id?: string;


  constructor(data?: Partial<Company>) {
    super(data);
  }
}

and then we also decide to create an "Address" model:

@model()
export class Address extends Entity {
  @property({
    type: 'string',
    id: true,
    generated: true,
  })
  id?: string;


  constructor(data?: Partial<Address>) {
    super(data);
  }
}

Using the generator to create a relation Company hasOne Address might look like this:

? Please select the relation type hasOne
? Please select source model Company
? Please select target model Address
? Foreign key name to define on the target model companyId
? Source property name for the relation getter (will be the relation name) address

So as a result of the generation the source property name for the relation becomes address and this overrides the current string property:

@model()
export class Company extends Entity {
  @property({
    type: 'string',
    id: true,
    generated: true,
  })
  id?: string;

  @hasOne(() => Address)
  address: Address;

  constructor(data?: Partial<Company>) {
    super(data);
  }
}