Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jhipster jdl reactive-ms fails e2e tests #14196

Closed
mraible opened this issue Mar 6, 2021 · 14 comments · Fixed by jhipster/jhipster-bom#219
Closed

jhipster jdl reactive-ms fails e2e tests #14196

mraible opened this issue Mar 6, 2021 · 14 comments · Fixed by jhipster/jhipster-bom#219
Labels
area: bug 🐛 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: neo4j theme: reactive ⚛️ Spring WebFlux $200 https://www.jhipster.tech/bug-bounties/
Milestone

Comments

@mraible
Copy link
Contributor

mraible commented Mar 6, 2021

Overview of the issue

When I wrote Reactive Java Microservices with Spring Boot and JHipster (using beta 1), all e2e tests passed. They do not pass with the current main branch.

Motivation for or Use Case

We should not break things between beta versions.

Reproduce the error

npm link after cloning the main branch. Then, create a microservices architecture:

jhipster jdl reactive-ms

Start all the relevant Docker containers.

cd store && jhmongoup
cd ../blog && jhneo4jup
cd ../gateway && jhkeycloakup && jhregistryup

Start all apps using ./gradlew.

Go into the gateway directory and run npm run e2e.

image

If you change the gateway to use Cypress, the errors still happen.

cd gateway
# change .yo-rc.json to use Cypress
rm -rf *
jhipster --with-entities
./gradlew
# new terminal
npm run e2e
Related issues

#14178

Suggest a Fix

Maybe it's caused by this change to Neo4j?

JHipster Version(s)

main branch. Yes, this is a regression.

@mraible mraible added $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ $200 https://www.jhipster.tech/bug-bounties/ theme: reactive ⚛️ Spring WebFlux theme: neo4j labels Mar 6, 2021
@mraible mraible mentioned this issue Mar 6, 2021
5 tasks
@pascalgrimaud
Copy link
Member

@mraible : I afraid it's related to migration to Spring Boot 2.4. For each minor version of Spring Boot, there are so much changes. That's why it's not so easy : remember Spring Boot 2.2 -> 2.3, and now 2.3 -> 2.4

@atomfrede
Copy link
Member

atomfrede commented Mar 6, 2021

Trying it manually results in an 404 for the post request. Looking into it.

EDIT Looks like something went wrong during generation.

@atomfrede
Copy link
Member

Did not get any further on saturday, as it seemed to work after everything was generated correctly. Will try again today.

@atomfrede
Copy link
Member

It seems from spring-data-neo4j 6.0.3 -> 6.0.4/6.0.5 a blocking call was added to ReactiveNeo4JTemplate. Clarifying with Michael and Gerrit if thats on purpose or not. What is strange, that our daily builds didn't notice this regression as it should be a general problem.

@mraible
Copy link
Contributor Author

mraible commented Mar 8, 2021

@atomfrede Can we downgrade spring-data-neo4j as a workaround?

@atomfrede
Copy link
Member

Looking into it right now. For gradle this works:

    implementation('org.springframework.data:spring-data-neo4j') {
        version {
            strictly '6.0.3'
        }
    }

But the protractor tests are failing (which are not related to backend things). Cypress is just working fine. Should we downgrade for now until the potential upstream is fixed?

image

@atomfrede
Copy link
Member

Data-neo4j 6.0.6 will be released next week containing the needed fix. Thanks @meistermeier for fixing it that fast.

@pascalgrimaud
Copy link
Member

don't hesitate to ping me once everything related to neo4j is fixed @atomfrede
I'm waiting this one for doing the release :)

@atomfrede
Copy link
Member

spring-data-neo4j:6.0.6 has been released. I have updated the version in bom, it works now when using reactive as the blocking call has been removed (:heart: @meistermeier for fixing it that fast)

@mraible
Copy link
Contributor Author

mraible commented Mar 17, 2021

I tried the steps in this issue and all tests pass!

  26 passing (32s)

Of course, I had to try a couple times. Because Protractor. 😅

@mraible
Copy link
Contributor Author

mraible commented Mar 17, 2021

Is this enough to take Neo4j out of beta? 🧐

@atomfrede
Copy link
Member

We have still the issue when using DTOs. Gerrit and me are working on it. The main reason is that the generated entities have cycles. This also makes neo slower than it has to be. But from my point of view we could either prevent DTOs + neo for now or document how to create a workaround (which is ~5 loc) for the current entity and relation structure.

@meistermeier
Copy link

Although this is already closed, I add my comment here:
Spring Data Neo4j does not has problems with cycles and Neo4j is not slow in those cases. But in a cyclic constellation we would have to create cascading / explorative queries in SDN. My opinion is that "always bidirectional" is not a great solution in general.
I think that we should give users the freedom to decide if they want to have a bidirectional definition in their domain or not. I prefer to keep it unidirectional as the standard value.
If people want to have bidirectional relationships, this have to be defined explicitly in the jdl e.g.

// create only Exam -> Question
relationship OneToMany {
  Exam{questions} to Question
}
// create backward reference from Question -> Exam
relationship ManyToOne {
  Question{exam} to Exam
}

(example taken from GH-13797)

The problem that were reported in the issue ^^ is rooted in the DTO creation as @atomfrede already pointed out.
SDN wants to persist everything "as it is" in the Java object graph but having the bidirectional relationships in combination with not fully hydrated DTOs leads to a constellation that gives SDN the instruction to remove relationships because the related entity list is empty.

@atomfrede
Copy link
Member

Thanks for clarification. I also think having it uni-directional makes it easier to understand.

@pascalgrimaud pascalgrimaud added this to the v7.0.0 milestone Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: bug 🐛 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: neo4j theme: reactive ⚛️ Spring WebFlux $200 https://www.jhipster.tech/bug-bounties/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants