jhipster / jhipster-core

JHipster Domain Language, used by JHipster UML and JDL-Studio to generate entities
Apache License 2.0
346 stars 116 forks source link

JDL microservice and entities keyword behavior #426

Open Tcharl opened 4 years ago

Tcharl commented 4 years ago
Overview of the issue

Behavior of these two JDL keywords are not that intuitive, also they does not cover all the use cases.

Motivation for or Use Case

My proposal would be to move the 'microservice' keyword into the 'application' section to be more flexible and change the behavior of certain combination

Reproduce the error

As of today, we can play with that two keywords in that way:

  1. Only using entities keyword
application {
  config {
    baseName ms
    applicationType microservice
  }
  entities TestA
}

application {
  config {
    baseName gw
  }
  entities TestA
}

entity TestA (testA) {
    id Long
    title String required
}

Current behavior

Generates the entity in the two backends, but gateway form only for gateway backend API

Expected behavior

Generates backends entity and forms for both backends, adding a textual differentiator in the 'Entities' frontend menu (ms/gw)

  1. Using the 'microservice' keyword for microservice
application {
  config {
    baseName ms
    applicationType microservice
  }
  entities TestA
}

application {
  config {
    baseName gw
  }
  entities TestA
}

entity TestA (testA) {
    id Long
    title String required
}

microservice TestA with ms

Current behavior

Generate backend in the ms, forms in the gw

Expected behavior Here, we do not have modified anything concerning the gw application configuration, but the behavior of the generation in that component changed: there's no more backend entity. Also, the frontend forms are now redirecting to the 'ms' backend instead of the 'gw' that was achieved in the former jdl. It would have make sense if the 'microservice' keyword was put within the application#gw object

  1. Using microservice for both components

If we finally use that configuration:

application {
  config {
    baseName ms
    applicationType microservice
  }
  entities TestA
}

application {
  config {
    baseName gw
  }
  entities TestA
}

entity TestA (testA) {
    id Long
    title String required
}

microservice TestA with ms
microservice TestA with gw

Actual behavior Here, it's even worse:

Expected behavior Not generating that configuration at all...

Conclusion

Covered use cases

As of today, we can

Uncovered use case Patterns like CDC/CQRS, which would need to generate backend in both services and has two frontends forms to call one or the other.

Suggest a Fix

Expected behaviors

  1. When we want no backend in the gateway
application {
  config {
    baseName ms
    applicationType microservice
  }
  entities TestA
}
application {
  config {
    baseName ms2
    applicationType microservice
  }
  entities TestA
}

application {
  config {
    baseName gw
  }
  entities TestA
  microservice TestA with ms, ms2
}

entity TestA (testA) {
    id Long
    title String required
}

Specifying the microservice attribute in the gateway application won't generate backend and frontend forms for the entity at the gateway level. It will only generate frontend forms calling the microservices

  1. When we want backend in the gateway

Specifying

application {
  config {
    baseName ms
    applicationType microservice
  }
  entities TestA
}

application {
  config {
    baseName gw
  }
  entities TestA
  microservice TestA with ms, gw
}

entity TestA (testA) {
    id Long
    title String required
}

And same expected behavior if we do not use the 'microservice' attribute. Generates the backend everywhere and the according frontends forms.

  1. skipClient

I would also suggest to move skipClient in the application section, that will permit to have a finer grain of control (generating client in a gateway but not on another), as well as permitting to generate backend on the gateway without generating it's according frontend.

JHipster Version(s)

6.6.0

JHipster configuration

Any configuration using microservice

MathieuAA commented 4 years ago

There seems to be more than one issue here, this makes it all kinda hard to read and understand. Give me time to parse everything

MathieuAA commented 4 years ago

Only using entities keyword

Generates the entity in the two backends, but gateway form only for gateway backend API

Generates backends entity and forms for both backends, adding a textual differentiator in the 'Entities' frontend menu (ms/gw)

I don't understand. What do you mean by "form"? Do you mean the visual form where the user can do the CUD actions? I really don't understand the issue...

Using the 'microservice' keyword for microservice

Generate backend in the ms, forms in the gw

Here, we do not have modified anything concerning the gw application configuration, but the behavior of the generation in that component changed: there's no more backend entity. Also, the frontend forms are now redirecting to the 'ms' backend instead of the 'gw' that was achieved in the former jdl.

It's weird the backend isn't generated in the GW.

It would have make sense if the 'microservice' keyword was put within the application#gw object

Stay tuned.

Using microservice for both components

The example should throw an error. Using the MS option with a value referencing a GW app should fail.

Suggestions

application {
  config {
    baseName ms
    applicationType microservice
  }
  entities TestA
}
application {
  config {
    baseName ms2
    applicationType microservice
  }
  entities TestA
}

application {
  config {
    baseName gw
  }
  entities TestA
  microservice TestA with ms, ms2
}

entity TestA (testA) {
    id Long
    title String required
}

Having an option declaration in the application "scope" is a feature that will be implemented. However microservice TestA with ms, ms2 in an application scope doesn't make much sense (especially if this option is in a gateway application scope).

 When we want no backend in the gateway

Why not use skipServer?

When we want backend in the gateway

It should be by default unless I'm missing something in this case.

skipClient

I would also suggest to move skipClient in the application section, that will permit to have a finer grain of control (generating client in a gateway but not on another), as well as permitting to generate backend on the gateway without generating it's according frontend.

Having the option be available everywhere allows to be more precise than just in the application scope, don't you think? What about a JDL file where there's no defined application but just entities. This suggestion might block a use case.

Tcharl commented 4 years ago

Sorry @MathieuAA for the multiple concerns here, didn't find any way to split it properly...

I don't understand. What do you mean by "form"? Do you mean the visual form where the user can do the CUD actions? I really don't understand the issue...

Yes, speaking about CRUD forms generated by JH to let the user manipulate entities (in the angular generator, everything's located in app/entities and app/shared/model

Having an option declaration in the application "scope" is a feature that will be implemented. However microservice TestA with ms, ms2 in an application scope doesn't make much sense (especially if this option is in a gateway application scope).

Microservice keyword would mean 'I want to be able to communicate with backend entities that are located there', meaning that we must generate that backend endpoints as well as the according frontend forms.

Why not use skipServer

Skipserver will also skip the frontend forms generation (as the endpoints won't be there) so I don't get that keyword, or at least it's not clear enough: that's why I was in favor of the 'microservice' keyword (microservice acting as a whitelist, skipServer being a blacklist, both covering the same area). Still, It can be a good candidate to replace the 'microservice' keyword as described previously: a combination of 'skipClient', 'skipServer ... with list of microservices and gateways' and 'entities' within an 'application' would suffice to cover everything if I'm not wrong. Up to the community to choose which one is the more meaningful

skipClient

I don't get a use case when there are entities without application: to me and following our private mailing list it should no longer be supported in the short term.

The current situation (having it out of the application object) is blocking the following use case:

application { // Here I want frontend and backend
  config {
    baseName gw
    applicationType gateway
  }
  entities TestA
}

application { // Here I want only backend
  config {
    baseName gw2
    applicationType gateway
  }
  entities TestA
  skipClient TestA
}

entity TestA (testA) {
    id Long
    title String required
}
MathieuAA commented 4 years ago

I think all this is a matter of vocabulary, so let's get this done first (sit tight, this is hair-pulling stuff, don't say I didn't warn you):

I'm not in favor of changing the meaning of a keyword. I like to add and deprecate (then delete) keywords, but changing them can have a heavy toll on the user. I'll read again what you wrote and think it over.

Thanks a lot for the explanations and the ideas. I think there's definitely something to be done here.

Tcharl commented 4 years ago

Let me give a try.

Would it make sense? we aren't modifying keyword meaning and not modifying the entire dsl that much. Doing so will let us handle most of the use cases

MathieuAA commented 4 years ago

A part of this issue will be resolved once options are available in the application scope (without breaking change, which is good). I've given much thought about this in the past few days (during the the implementation of options in applications) and rethinking how options are handled at multiple levels should be done right after and documented.

Tcharl commented 4 years ago

Sure :-), I'm also following the application scope issue! What about

  1. exchanging on this or multiple tickets if we can split the problematics
  2. Agree between community members
  3. Implements and only merge when the according documentation is here