Call methods in templates instead of creating properties in entity jsons

[andidev]

Instead of creating json properties to hold data needed in the templates we should create methods that can be called from the templates to get the data wanted.

The property fieldNameCapitalized, fieldNameUnderscored, fieldInJavaBeanMethod is today stored in each field, e.g.

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldNameCapitalized": "Data",
      "fieldNameUnderscored": "data",
      "fieldInJavaBeanMethod": "Data",
      "fieldValidate": false,
      "fieldType": "String",
      "fieldIsEnum": false
    }
  ],

and then used in the templates as e.g.

    @Field("<%=fields[fieldId].fieldNameUnderscored %>")<% } %>
    private <%= fields[fieldId].fieldType %> <%= fields[fieldId].fieldName %>;

instead of using properties we should expose the methods in the template context that does the capitalization, etc. e.g.

    @Field("<%=underscored(fields[fieldId].fieldName) %>")<% } %>
    private <%= fields[fieldId].fieldType %> <%= fields[fieldId].fieldName %>;

This will make the entity jsons a lot more cleaner.

I think we can replace the following properties with methods as well

    "fieldsContainOwnerManyToMany": false,
    "fieldsContainOwnerOneToOne": true,
    "fieldsContainOneToMany": false,
    "fieldsContainLocalDate": false,
    "fieldsContainCustomTime": false,
    "fieldsContainBigDecimal": false,
    "fieldsContainDateTime": false,
    "fieldsContainDate": false,
    "fieldsContainBlob": false,

[gmarziou]

Unless I misunderstood your proposition, I can see 2 cases where it would not work:

• someone wants to customize how a field name is underscored
• some tool needs to access the information after generation (maybe jhipster uml, I haven't looked at it yet)

[deepu105]

@andidev I too think its good to have the flexibility to change the names in the json, but the second part concerning the dates are annoying as it causes error if you forget to change when modifying any json by hand so if that can be avoided that will be great i guess. as @gmarziou said we should think of Jhipster UML as well @MathieuAA any comments on this?

[andidev]

The customization is a strong argument. Did not think of that. Though I'm not sure why the user would ever want to change the fieldNameCapitalized and fieldInJavaBeanMethod properties. Note that the field name would still be modifiable and therefore the fieldNameCapitalized and fieldInJavaBeanMethod property values as well. The only thing that is not modifiable is how the capitalization and java bean capitalization is done. But most probably I am wrong and there is some use-case where one needs to change them.

One thing for sure is that at least we could replace

    "fieldsContainOwnerManyToMany": false,
    "fieldsContainOwnerOneToOne": true,
    "fieldsContainOneToMany": false,
    "fieldsContainLocalDate": false,
    "fieldsContainCustomTime": false,
    "fieldsContainBigDecimal": false,
    "fieldsContainDateTime": false,
    "fieldsContainDate": false,
    "fieldsContainBlob": false,

with methods instead since the information is in the json. The only problem is that you have to loop through it to find the information. So any external tool can still access it.

Awaiting @MathieuAA to comment this.

[MathieuAA]

First, let me just say that this would be a great improvement!

From a JHipster-UML-y point of view, @CarlKlagba and I would really like to remove the code that generates things like "fieldNameCapitalized", or "fieldNameUnderscored". It's pretty straightforward to make this change (as far as I can see, only code removal is needed, no addition whatsoever).

However, having played with the templates a bit, I must say that changing them would be quite painful... especially for the Entity sub-generator.

Having said that, here's my opinion:

• As @gmarziou said, if one wants to customise how a name is underscored (even if I don't see it happening in a practical world), we may have a problem... An "easy" solution would be to make it "optional" (if !XXXUnderscored). If it's possible, of course!
• To Gaël: We (the JHipster-UML team) would not have any problem removing these fields.
• @deepu105, concerning the dates, yes, it could be troublesome down the road, but an easy way must exist (an easier way than what is currently implemented), we "just" have to think of it.
• @andidev, I agree, and it may be a good reason not to change it (looping is bad)... unless we completely change how the JSON is generated, but this solution could be worse than the problem.

Frankly, I would love to break everything (the JSON generation code), take only the good parts, and build a better JSON generating system. But that's what I want. @CarlKlagba, do you have an input on this?

[gmarziou]

In some languages capitalization like pluralization can be a complex task that our simple generator may always get wrong because it does not know the developer's locale.

[gmarziou]

@MathieuAA regarding painful templates, did you read my proposition in #1749? Do you think it would have helped you?

[CarlKlagba]

I am of @andidev opinion, we should remove properties in the JSON file. And I am with you @MathieuAA about the customization, we could make some fields optional, with a simple existence test in the function.

About the way to access the information after generation, with JHipster-UML, our only interface with the JHipster app is the JSON, so we unfortunately do not have the tool.

I talked I about it 2 or 3 mouths ago with @jdubois, - I don't want to put words in his mouth but- he told me that a user did a PR to create the JSON, and they were simple memory dump, and he never really took the time to optimize it.

I think we should re-think it to make it more human friendly, but in the meantime you can still use JDL ! =)

[gmarziou]

Just made some tests, the entity generator prevents you from using special characters in field names, so the risk for having incorrect capitalization is probably null.

[MathieuAA]

@gmarziou, I've read what you wrote, and I liked the idea (just didn't have the time to comment). Anything to make any generator less problematic to handle is (generally) a good idea. And yes, it would have helped me. Both ideas (the one exposed here and your previous idea) are good to take.

[deepu105]

@andidev @gmarziou yes it would be nice if we can remove this

"fieldsContainOwnerManyToMany": false,
    "fieldsContainOwnerOneToOne": true,
    "fieldsContainOneToMany": false,
    "fieldsContainLocalDate": false,
    "fieldsContainCustomTime": false,
    "fieldsContainBigDecimal": false,
    "fieldsContainDateTime": false,
    "fieldsContainDate": false,
    "fieldsContainBlob": false,

without having to loop, but even if we loop i dont think thats a big deal given these json are not that huge, we can find types from the fieldType and do imports accordingly. Regarding the naming fields i was wondering the same why would someone need both fieldNameCapitalized and fieldInJavaBeanMethod I guess we can make the json more readable by using something like below, even the fieldIsEnum seems unwanted

"fields": [
    {
      "fieldId": 1, // is this even used anywhere?
      "fieldName": "data",
      "fieldNameCapitalized": "Data", // i guess this would be modified very rarely
      "fieldNameUnderscored": "data",  // this might be modified often
      "fieldValidate": false,
      "fieldType": "String"  // enum type should be put here
    }
  ],

@MathieuAA @CarlKlagba I loved the idea of JDL may be we can make it and the entity json very similar in structure, I was thinking of doing a visual editor for JDL like this - http://deepu105.github.io/svg-seq-diagram/ and this - http://deepu105.github.io/svg-seq-diagram/plantUML.html , just need to find some time. Your JDL structure is very similar to plant uml structure

[CarlKlagba]

@deepu105 Woo ! That looks great. If we could also do the opposite we could use it to make a class diagram editor.

[gmarziou]

Nice changes but it seems to be a breaking compatibility change, so should it be done on 3.0 branch?

[andidev]

@gmarziou , @deepu105 , @MathieuAA , @CarlKlagba So after reading all answers maybe I have a solution that will make everybody happy and even solve issue #1754 if we do it correctly.

So what if we do it like this:

By default stop saving redundant information into the entity json files. This means

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldNameCapitalized": "Data",
      "fieldNameUnderscored": "data",
      "fieldInJavaBeanMethod": "Data",
      "fieldValidate": false,
      "fieldType": "String",
      "fieldIsEnum": false
    }
  ]

would be saved as

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldValidate": false,
      "fieldType": "String"
    }
  ]

instead when running the generator, generate the missing data into the json object available in the template context so it looks as it should (without saving it to the entity json file of course)

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldNameCapitalized": "Data",
      "fieldNameUnderscored": "data",
      "fieldInJavaBeanMethod": "Data",
      "fieldValidate": false,
      "fieldType": "String",
      "fieldIsEnum": false
    }
  ]

but only load data that does not already exists in the entity json object saved to file, if one would want to override the data one could then easily do that by adding e.g. "fieldNameCapitalized": "DATA", to the entity json file

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldNameCapitalized": "DATA",
      "fieldValidate": false,
      "fieldType": "String"
    }
  ]

this solution partially fixes #1754 since it solves the issue when fieldInJavaBeanMethod": "Country" is missing from the entity json file. Though to make this solution fully solve the issue lets also make sure to add default values to data that actually should be included in the entity json file e.g. "fieldValidate": false. e.g.

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldType": "String"
    }
  ]

would still be valid and when after loading the data that will be available in the template context it would look like it should

  "fields": [
    {
      "fieldId": 1,
      "fieldName": "data",
      "fieldNameCapitalized": "Data",
      "fieldNameUnderscored": "data",
      "fieldInJavaBeanMethod": "Data",
      "fieldValidate": false,
      "fieldType": "String",
      "fieldIsEnum": false
    }
  ]

though lets make sure that the generator maybe outputs an warning that the property is missing.

This would be a quite small change and it would only affect the index.js file and no template files. It would not break any backward compability. It would still allow the flexibility to override entity json properties. It would make the entity json files a lot more readable and easier to modify. It would make it a lot easier to update entities with newer jhipster versoins. Looping is only done once when loading the missing data. The only drawback that I can see is that overriding properties are a bit more difficult since it is not clear which properties exists to override, but this could be documented.

Applying this change would reduce the json file from e.g.

{
  "relationships": [
    {
      "relationshipId": 1,
      "relationshipName": "child",
      "relationshipNameCapitalized": "Child",
      "relationshipFieldName": "child",
      "otherEntityName": "child",
      "otherEntityNameCapitalized": "Child",
      "relationshipType": "many-to-one",
      "otherEntityField": "id"
    }
  ],
  "fields": [
    {
      "fieldId": 1,
      "fieldName": "firstName",
      "fieldNameCapitalized": "FirstName",
      "fieldNameUnderscored": "first_name",
      "fieldInJavaBeanMethod": "FirstName",
      "fieldValidate": false,
      "fieldType": "String",
      "fieldIsEnum": false
    }
  ],
  "fieldsContainOwnerOneToOne": false,
  "fieldsContainOwnerManyToMany": false,
  "fieldsContainOneToMany": false,
  "fieldsContainLocalDate": false,
  "fieldsContainCustomTime": false,
  "fieldsContainBigDecimal": false,
  "fieldsContainDateTime": false,
  "fieldsContainDate": false,
  "changelogDate": "20150329233141",
  "dto": "no",
  "pagination": "no",
  "validation": false
}

to

{
  "relationships": [
    {
      "relationshipId": 1,
      "relationshipName": "child",
      "otherEntityName": "child",
      "relationshipType": "many-to-one",
      "otherEntityField": "id"
    }
  ],
  "fields": [
    {
      "fieldId": 1,
      "fieldName": "firstName",
      "fieldValidate": false,
      "fieldType": "String"
    }
  ],
  "changelogDate": "20150329233141",
  "dto": "no",
  "pagination": "no"
}

Hope this makes sense.

[deepu105]

@andidev this is def better than the current impl, +1 from me

[MathieuAA]

If I understand correctly, you'd have an object in memory have all the needed info, but write a JSON that only contains what's really useful (and that can be overriden if needed), so that everything else could be deduced from it. You have my +1.

[deepu105]

@andidev I guess if you are gonna attempt this then ill wait as I was thinking of doing this issue https://github.com/jhipster/generator-jhipster/issues/1721

[andidev]

You can do #1721 if you want. I have to finish #1468 first.

[jdubois]

Just to confirm: yes the current format wasn't something I thought of, so it definitely needs to be refactored.

Let me give you some explanation:

• At the beginning, I was doing everything in memory, and I wasn't saving anything in a JSON file.
• Then we had this great PR where someone was basically dumping the memory to a JSON file, and then we were able to re-run the generator again, that was totally unexpected!
• This allowed us to re-run the generator, to create JHipster UML, etc... So that was really great

But of course, this is just the memory dump of something that wasn't meant to be saved to a JSON file! So if you can improve it, feel free to do it!

[gmarziou]

I'm developing a mobile hybrid app on top of Cordova/Ionic for few weeks and that will use JHipster as a backend, I am thinking about prototyping a separate ionic entity sub generator that would read the json format and generate ionic client side in another module. So anything that could simplify this entity JSON format would help.

[andidev]

@gmarziou Interesting, I have started working on it. I'll probably provide a PR next week.

[gmarziou]

Great

[andidev]

So I think my work here is finally done.

Heres a list of the json properties that has been moved out from the json file into the memory:

relationship level

• relationshipNameCapitalized
• relationshipFieldName
• otherEntityNameCapitalized

field level

• fieldNameCapitalized
• fieldNameUnderscored
• fieldInJavaBeanMethod
• fieldIsEnum
• fieldValidate

root level

• fieldsContainOwnerManyToMany
• fieldsContainOneToOne
• fieldsContainOwnerOneToOne
• fieldsContainOneToMany
• fieldsContainLocalDate
• fieldsContainCustomTime
• fieldsContainBigDecimal
• fieldsContainDateTime
• fieldsContainDate
• fieldsContainBlob
• validation
• pagination (MOVED ONLY FOR CASANDRA DB)

I've also added some validation to the json properties that are needed. If a property of importance is missing the generator will output e.g.

ERROR fieldName is missing in .jhipster/MyEntity.json for field with id 2

and exit the generator. For new properties that can have fallback values (e.g. when adding new features) a warning is displayed if the property is missing, e.g.

WARNING dto is missing in .jhipster/MyEntity.json, using no as fallback

Also it is possible to override any of the following in-memory properties by adding them to the json file:

relationship level

• relationshipNameCapitalized
• relationshipFieldName
• otherEntityNameCapitalized

field level

• fieldNameCapitalized
• fieldNameUnderscored
• fieldInJavaBeanMethod

I have done some heavy testing where I tested to generate all field types and relationships available for SQL and CASANDRA databases. I have compared the output of the generated source code before changes with after changes and they are the same. So the code should be ok to merge.

@MathieuAA , @CarlKlagba If you have any questions don't hesitate to ask. Is there any work on your side to do before we release these changes?

@jdubois I guess we should wait merging this after the next release?

[deepu105]

+1 @andidev Can you plz test with enableTranslation:false as well just to make sure. and may be refactor the npm tests as well accordingly if needed. I guess the relationshipFieldName property will be overriden if user chooses that in entity generator rite?

[MathieuAA]

We will test it on our side and do some JSON cleaning, it should take long.

[andidev]

@deepu105 enableTranslation:false should not be a problem since it is a global project property living in .yo-rc.json. relationshipFieldName property is never asked in the entity generator rite and therefore it is not overridden. Though it could be overridden if wanted in the entity.json file.

[cbornet]

@gmarziou for your ionic client, you could probably make a swagger-codegen client. That would interest a lot of people I think.