Add elasticsearch option when mongo is selected

[ramzimaalej]

This is a first attempt to fix this issue #1810

@jdubois @deepu105 Let me know if you have any feedback on this. I also would like to add tests to this and solve this non-blocking issue:

org.reflections.ReflectionsException: could not create Vfs.Dir from url, no matching UrlType was found [file:/Users/rmaalej/Documents/dev/workspace/expensy/expensy-analytics/build/classes/scala/main] either use fromURL(final URL url, final List<UrlType> urlTypes) or use the static setDefaultURLTypes(final List<UrlType> urlTypes) or addDefaultURLTypes(UrlType urlType) with your specialized UrlType. at org.reflections.vfs.Vfs.fromURL(Vfs.java:109) at org.reflections.vfs.Vfs.fromURL(Vfs.java:91)

[pascalgrimaud]

Congrats @ramzimaalej !
It's a feature a lot of people wanted since 2 years, but no one contribute to it.

[jdubois]

Yes, awesome!

[deepu105]

To make it less ugly when using mongo without ES I suggest to create an intance variable called mongoDocumentAnnotation and use it here the value of which can be qualified path when using ES and just document when using without ES. The same gies for ES annotation and the imports

[ramzimaalej]

sure, I will take care of it

[deepu105]

While this is correct my idea was to simplify this with a variable. Dont worry ill do it while merging. Ill merge this later

[ramzimaalej]

I think this way offers better readability of what's going in the template unless you want to put the logic here, but it's up to you. I don't have a strong opinion about it.

[deepu105]

Please check the test failure for new setup

[ramzimaalej]

@deepu105 I'm looking into the issue, it's related to mapstruct bean not being injected. How many people are using Mapstruct. I'm thinking of disabling Mapstruct when mongo and elasticsearch are selected, until we figure out the issue. @jdubois @deepu105 thoughts?

[ramzimaalej]

Intellij was misleading me, the issue has nothing to do with Mapstruct. Actually, it's because of the ZonedDateTime converter.

`public static class DateToZonedDateTimeConverter implements Converter<Date, ZonedDateTime> {
public static final JSR310DateConverters.DateToZonedDateTimeConverter INSTANCE = new JSR310DateConverters.DateToZonedDateTimeConverter();

    private DateToZonedDateTimeConverter() {
    }

    public ZonedDateTime convert(Date source) {
        return source == null ? null : ZonedDateTime.ofInstant(source.toInstant(), ZoneId.systemDefault());
    }
}

public static class ZonedDateTimeToDateConverter implements Converter<ZonedDateTime, Date> {
    public static final JSR310DateConverters.ZonedDateTimeToDateConverter INSTANCE = new JSR310DateConverters.ZonedDateTimeToDateConverter();

    private ZonedDateTimeToDateConverter() {
    }

    public Date convert(ZonedDateTime source) {
        return source == null ? null : Date.from(source.toInstant());
    }
}`

while technically speaking the generated object represent the same timestamp, they are slightly different causing this line to fail

assertThat(fieldTestMapstructEntityEs).isEqualToComparingFieldByField(testFieldTestMapstructEntity);

I have changed to use string instead of data and it works like a charm

`public static class StringToZonedDateTimeConverter implements Converter<String, ZonedDateTime> {
public static final StringToZonedDateTimeConverter INSTANCE = new StringToZonedDateTimeConverter();

    private StringToZonedDateTimeConverter() {
    }

    public ZonedDateTime convert(String source) {
        return source == null ? null : ZonedDateTime.parse(source);
    }
}

public static class ZonedDateTimeToStringConverter implements Converter<ZonedDateTime, String> {
    public static final ZonedDateTimeToStringConverter INSTANCE = new ZonedDateTimeToStringConverter();

    private ZonedDateTimeToStringConverter() {
    }

    public String convert(ZonedDateTime source) {
        return source == null ? null : source.toString();
    }
}`

I raised a PR on jhipster core: jhipster/jhipster#31

[cbornet]

The issue I see is that ES stores Date objects as Long so using a String will be less preformant and compact.
The issue with isEqualToComparingFieldByField() is that it uses the equals() method which checks the timezone of ZonedDateTime but as we converted to Date before storage we lose this information. We have a matcher TestUtil.sameInstant for the other tests that checks ZonedDateTime ignoring the timezone.
So I think it would be better to use isEqualToIgnoringGivenFields , ignoring the ZonedDateTime fields and use individual sameInstant matchers for the remaining ZonedDateTime fields.

[ramzimaalej]

If we use long we will also lose the timezone, because when you create a ZonedDateTime from an Instant you need to specify the Timezone. That said, there is no way to guess the original timezone. I would rather use a string representation that preserves the original timezone and avoids adding complexity to the generator to find out which fields that are ZonedDateTime to be treated differently in assert statement.

[cbornet]

Most of our DBs (currently all of them except cassandra) don't preserve the timezone and the view layer will remove it anyway so it's not a big deal to lose it.

[ramzimaalej]

done. let me know if there is anything else to address in this PR.

[ramzimaalej]

@deepu105 @jdubois do you guys feel there is more stuff to do here?

[jdubois]

I just need time to review but this looks quick to do

[pascalgrimaud]

LGTM
Just give a quick try, and it works well. Good job @ramzimaalej

• generated a project with MongoDB + ES
• generated some entities
• launched app
• played with entities + ES

So I don't know if this PR needs to be merged jhipster/jhipster#31

[ramzimaalej]

@pascalgrimaud I think we can discard it for now (jhipster/jhipster#31).

[ramzimaalej]

@pascalgrimaud can you please restart the build.

[pascalgrimaud]

it's green, so it's ok ?

[ramzimaalej]

Yeap. Once merged I will work on adding elasticsearch to couchbase as well.

[ramzimaalej]

@deepu105 @jdubois any plans on when this will get merged?

[jdubois]

@ramzimaalej not before the next release, where we modified our build system (with jhipster/jhipster-dependencies) and added Couchbase support. This is already a huge release, and we need some time to iron it out.
So that would be the release after.

[deepu105]

This needs to be rebased as the entity templates are moved to entity-client and entity-server folders

[ramzimaalej]

@deepu105 done

[pascalgrimaud]

@ramzimaalej : just for your information, as soon as it's merged, I will probably move your Travis tests to https://github.com/hipster-labs/jhipster-travis-build because I'm not sure MongoDB + Elasticsearch will be used a lot. And I will add MongoDB + Elasticsearch + Gradle tests too.

I don't request change here as you already work a lot on this.
Thanks for your patience!

[ramzimaalej]

no prob @pascalgrimaud Let me know if you need help.

[jdubois]

LGTM and @deepu105 your changes were corrected, so let's merge this.
Congrats @ramzimaalej on a huge new feature!