Generator-jhipster: Subobjects inserted with null values with Elasticsearch and DTOs

[stieler-it]

Overview of the issue

While creating a entity, it is saved both to the database and to Elasticsearch by the EntityServiceImpl's save method. While creating an entity, all sub-objects are only saved with their ID, all other fields are null. When the entity is updated, all fields are present.

Motivation for or Use Case

This results into an incomplete state in Elasticsearch and can even produce functional problems, e.g. when the search should include fields of linked entities.

Reproduce the error

1. Create a new entitiy
2. Look into Elasticsearch index or try to search for content present in a field of a linked entity.

Related issues

n/a

Suggest a Fix

We found out that:

1. While creating an object, after entity = einsatzortRepository.save(entity); entity still contains incomplete fields (as it was received / mapped from the DTO)
2. While updating an object, after entity = einsatzortRepository.save(entity); entity has been filled from the database

We only have a workaround for our bug and did not come up with a real idea for a fix yet. The workaround is: If the EntityResource calls save() on the service twice, the data is persisted in Elasticsearch correctly.

JHipster Version(s)

4.6.2

JHipster configuration

.yo-rc.json:

{
  "generator-jhipster": {
    "promptValues": {
      "packageName": "de.XXXX",
      "nativeLanguage": "de"
    },
    "jhipsterVersion": "4.6.2",
    "baseName": "xxxx",
    "packageName": "de.XXXX",
    "packageFolder": "de/XXXX",
    "serverPort": "8080",
    "authenticationType": "oauth2",
    "hibernateCache": "no",
    "clusteredHttpSession": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "h2Memory",
    "prodDatabaseType": "oracle",
    "searchEngine": "elasticsearch",
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSocialSignIn": false,
    "clientFramework": "angularX",
    "useSass": true,
    "clientPackageManager": "yarn",
    "applicationType": "monolith",
    "testFrameworks": [
      "protractor"
    ],
    "jhiPrefix": "jhi",
    "enableTranslation": true,
    "nativeLanguage": "de",
    "languages": [
      "de"
    ]
  }
}

Entity configuration(s) entityName.json files generated in the .jhipster directory

main.json

{
    "fluentMethods": true,
    "relationships": [
        {
            "relationshipType": "many-to-one",
            "relationshipValidateRules": "required",
            "relationshipName": "otherEntity",
            "otherEntityName": "otherentity",
            "otherEntityField": "name"
        }
    ],
    "fields": [
        {
            "fieldName": "name",
            "fieldType": "String",
            "fieldValidateRules": [
                "required",
                "minlength",
                "maxlength",
                "pattern"
            ],
            "fieldValidateRulesMinlength": 2,
            "fieldValidateRulesMaxlength": 50,
            "fieldValidateRulesPattern": "^[A-ZÄÖÜa-zäöüß\\- ]*$"
        },
       // ,,,
    ],
    "changelogDate": "20170704124210",
    "entityTableName": "eso",
    "dto": "mapstruct",
    "pagination": "infinite-scroll",
    "service": "serviceImpl"
}

Browsers and Operating System

Unrelated.

• [X] Checking this box is mandatory (this is just to show you read everything)

[cbornet]

Not sure about this. You would pull the entire entity graph...

[gzsombor]

It's a known issue - and a problematic one, as JPA/Hibernate could make nasty things behind your back, because it creates the proxy objects, and this could end up in strange JSON serialization problems - sometimes an exception happens, sometimes it's "just" data loss.
And the last problem, is the transactionality - if you have a slighlty more complicated service layer, where exception could happen, then you need to be very careful, when to put your data into ES.
From my experiences with JHipster + ElasticSearch + JPA, I would suggest the following:

• Use separate classes for the ES entities, so if you have an Order and OrderLine JPA entities, do create a _OrderDocument_ / _OrderLineDocument_ classes for ES - so you can easily control, what fields and objects are serialized to ES index. (You need mapper classes for them as well)
• With the help of _org.springframework.transaction.support.TransactionSynchronizationManager_ you can register callbacks like this: TransactionSynchronizationManager.registerSynchronization( ... afterCommit() { .... indexEntityById(id); } )
  The important part is, to load everything from JPA by ID, and not passing the whole entity - this way you will get pristine, persistent entities, which child entities consistently filled from database, so you can convert it into ES objects.
• If you want, you can play with different levels of asynchronity - for example the 'afterCommit()' could delegate the actual indexing into a separate processing thread - this way, you could return to the user faster - obviously, the indexing will happens in the background.
• Generally, if you have lot's of entities referencing each other, with deep ES indexes, you will need a minimal message bus, or some queue so you can delegate re-indexing tasks to the background.

Unfortunately, I think, there is no simple and generic solution exists handling JPA + ES together, which works well with a more complex object model. I can imagine, JHipster could generate these separate _DocumentEntities_ and _DocumentEntityMappers_ - maybe I had suggested this before, but there are many people, who thinks that we generate too much code, which is a bit overwhelming at first. Sometimes I could understand that.

[stieler-it]

Well, we see JHipster as a way to quickly bootstrap a working full stack application. And I totally agree that for a real world application a lot can/should be customized in terms of features or even security. (And thanks a lot for the hints, we might need them!) However, in my opinion, JHipster should try to make sure that provided features are "reliable".

That means, first we wondered why every object is stored in ES together with subobjects. Then, we liked it because it allowed us to have a better search experience without coding ourselves. Today we noticed that this nice feature only works _sometimes_.

So, I would prefer generated code that works and can be extended or replaced over code that has to be bugfixed. Probably even a slim ES feature without sub objects would be a good start?

[stieler-it]

Since this problem came up again we had to find a solution without re-writing the search functionality from scratch.

Basicall, in every Mapper, instead of:

    default Office fromId(Long id) {
        if (id == null) {
            return null;
        }
        Office office = new Office();
        office.setId(id);
        return office;
    }

we now have:

    public Office fromId(Long id) {
        if (id == null) {
            return null;
        }
        return officeRepository.findOne(id);
    }

This probably means some database calls that would not be necessary. However, every saved entity contains all its nested objects. This will save as from a bunch of bugs that would have been reported otherwise.

What do you think?

[jdubois]

The problem for us is that it's very hard to find a generic solution, that would be 1) simple to understand & modify and 2) doesn't have this kind of bugs
@stieler-it there is one thing I don't get: I understand you have this issue with DTOs, but do you also have this without DTOs?

[stieler-it]

That is a good question. How are subobjects sent from the frontend when not using DTOs? Is the wohle object sent and received, not just the IDs? Are the subobject saved to ES "as-is" or are they refreshed/verified from the database? I have currently only one active JHipster project which is using DTOs so I can't tell.

I think my last suggestion is both simple to understand and gets rid of the bugs. However it means a lot of probably unnecessary database calls.

[deepu105]

@stieler-it @jdubois so what can we do here? its been open a for a while

[stieler-it]

I think we have three "clean" options:

1. Always save nested objects (e.g. using my approach fetching all subobjects on each save)
2. Never save nested objects (e.g. using @JsonIgnore for subobjects)
3. Document how Elasticsearch with DTOs is kind of broken and let users write own code according to their (probably very individual) needs

[ruddell]

Do you have an entity configuration or JDL that will reproduce the issue? Maybe I'm just not seeing it. Here's example output https://gist.github.com/ruddell/168035f7c9d3724c13c562446a86f9fc

Without DTOs, the subobject is indexed correctly and returned on the ManyToOne side but not on the OneToMany side (which is not eagerly loaded by default). So it works as expected when not using DTOs

[stieler-it]

Wer are using DTOs so I can't tell about other options. Glad to hear the scope of the problem is limited, though.

[Tcharl]

If you choose the solution 1, it should be nice to use Mapstruct resolver to fetch data, also, I you want it, ask me for a PR if needed ;-):

@Mapper(componentModel = "spring", uses = { XMapperResolver.class})
public class XMapper {
X fromId(Long id);
}
@Component
public class XMapperResolver {

    XRepository xRepository;

    public XMapperResolver(XRepository xRepository) {
        this.xRepository = xRepository;
    }

    @ObjectFactory
    public X resolve(Long id, @TargetType Class<X> type) {
        return Optional.ofNullable(id).map(i -> xRepository.findById(i).orElseGet(null)).orElse(null);
    }
}

[jdubois]

This has been opened for very long, and we haven't found a good solution - besides, very few people seem interested by the issue... For me the best solution would be to document it, as I don't see how we could have something generic here - at some point you need to code stuff, it's not possible to generate everything (unless we have a lot of people interested, and who code something really smart).

I'm closing this as for me this is stuck - if anyone has any solution, especially if that's just documentation, please do a PR on the documentation website and link it here!

[sdyson]

@stieler-it, thanks for the suggested workaround to lookup the entity in the Mapper. How do you inject the Repository into the Mapper, given it is an interface and the implementation is generated at build time?

[stieler-it]

@sdyson as far as I remember we had abstract classes as mappers. Most of the mapping logic was generated at build time, however we could still provide methods for generating entity classes there. I just asked Google and there seems to be another interesting way: https://mapstruct.org/documentation/stable/reference/html/#object-factories

[gzsombor]

I think, my suggestion from 2017 still valid : https://github.com/jhipster/generator-jhipster/issues/6410#issuecomment-332315624

[sdyson]

@stieler-it thanks, I figured out the same solution using abstract classes for the mappers. So far it works well for the few entities I have implemented, but I have over 40 entities so it would take a while to implement all of them. Given the solution seems to work well it is a shame this issue has been closed rather than implemented what seems to be a fairly simple solution.

[sdyson]

@gzsombor, I think your suggestion of having separate mappers to finely control what is indexed in ES is a very good idea. I understand the reservation about increasing the amount of generated code, but then isn't that also what is so great about JHipster? As long as the code is well architected, easy to understand and extensible then I really don't mind how much code it generates. The more custom code I have to write the more difficult it is for me to regenerate the code when my entity model changes.

[stieler-it]

@sdyson

but I have over 40 entities so it would take a while to implement all of them

I am pretty sure that we implemented a small template in our (private) fork of JHipster that did exactly this. Unfortunately I don't have access to the code any more.

[pascalgrimaud]

@sdyson : I understand the need, but if you read the previous comment, it has been closed simply because no one wants to try to implement something. So maybe it's minor or people don't care about this issue.

Anyway, you're welcome to propose a solution here :)