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

Spring Boot 2.0.0.M3 #6274

Closed
wants to merge 10 commits into from
Closed

Conversation

JoffreyDcu
Copy link

I did the following:

  • updated spring boot dependency
  • updated classes which used renamed or deprecated spring classes or functions

The binding of properties fails with Travis, but works fine locally.
ElasticSearch does not start a node by default anymore (use docker?).

  • Please make sure the below checklist is followed for Pull Requests.

  • Travis tests are green

  • Tests are added where necessary

  • Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

updated springtemplateengine version for mailservice
added missing Optional (not compulsory but that way we have the same code for mongo and mysql)
added optional in persistentremembermeservice
updated deprecated code
@PierreBesson
Copy link
Contributor

Maybe this could be merged on a new branch named jhipster-v5 or reactive as Spring Boot 2.0 is only scheduled for the end of november (at best !), see https://github.com/spring-projects/spring-boot/milestones. Then @JoffreyDcu and others would be free to work on reactive support on this branch.

@deepu105
Copy link
Member

@PierreBesson yes these should be worked on a new branch similar to jh-react

@deepu105
Copy link
Member

use this branch https://github.com/jhipster/generator-jhipster/tree/jh-reactive-spring

@deepu105 deepu105 changed the base branch from master to jh-reactive-spring August 22, 2017 10:02
@cbornet
Copy link
Member

cbornet commented Aug 24, 2017

Starting JDK9 support would also be nice !

@cbornet
Copy link
Member

cbornet commented Aug 25, 2017

So we move to spring-data-cassandra ?

@jdubois
Copy link
Member

jdubois commented Aug 25, 2017

@cbornet sorry yes, let me explain.

  • Less than 2% of users use Cassandra, so my own custom code brings a lot of maintenance for too few people
  • Spring Data Cassandra has been totally re-worked: it's supposed to be much better, but I haven't looked at it in detail
  • This is going to help us for Spring Webflux, as they already have reactive interfaces (which is not the case with my current code)

@balbler
Copy link

balbler commented Aug 27, 2017

I am also in the process of converting an older jhipster based app to Spring boot 2.0, so I will try to help with my limited knowledge.
Two Questions:
->ES 5.0 introduces new field types by splitting the former string type into a text and a keyword type (https://www.elastic.co/blog/strings-are-dead-long-live-strings) . Does it make sense to pull this differentiation into jhipster?

-> on my Application I do have an issue where there seems to be an issue with AbstractRememberMeServices causing NullPointerExceptions in the loginSuccess Method. Seems the Spring proxy is used there resulting in Null fields. Does not happen with older spring boot/ spring versions. I can reproduce this by also changing the jhipster sample app. Anybody else seeing this?

UPDATE:
On the second issue: Reason for this was that Spring boot 2.0 uses CGLIB proxies by default (https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.0.0-M1-Release-Notes)
this then causes this issue for the final method in AbstractRememberMeServices.
if i add

spring:
   aop:
            proxy-target-class: false

things work again. @jdubois : i am not really sure if this is the way to go? This basically now makes the issue as described in #763 (comment) occur by default. I will try to find something in the Spring boot Docus.

fixed error in CassandraConfiguration
@agoncal agoncal mentioned this pull request Sep 28, 2017
1 task
@deepu105
Copy link
Member

@JoffreyDcu I suggest doing the Spring boot update and Reactive support in 2 different PR so that its easy to review and merge
Also i'm not sure if adding Cassandra support is worth the effort for Reactive support

authorities.add(authorityRepository.findOne("ROLE_USER"));

Optional<Authority> authorityOptional = authorityRepository.findById("ROLE_USER");
if (!authorityOptional.isPresent()) throw new NoSuchElementException();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use orElseThrow

@agoncal
Copy link
Contributor

agoncal commented Dec 4, 2017

FYI I started working on integrating HibernateSearch in JHipster. But if we want to have all the nice Spring Boot Starter integration, we will have to wait for Spring Boot 2 : snowdrop/spring-data-example#2 (comment)

@@ -130,7 +147,7 @@
* @return the ResponseEntity with status 200 (OK) and the list of <%= entityInstancePlural %> in body
*/
@GetMapping("/<%= entityApiUrl %>")
@Timed<%- include('../../common/get_all_template', {viaService: viaService}); -%>
@Timed<%- include('../../common/get_all_template', {viaService: viaService}) -%>
Copy link
Member

@cbornet cbornet Dec 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reactive option, we should probably also have an endpoint producing application/stream+json to prevent blocking in large JSON serialization/deserialization

@deepu105 deepu105 added this to In progress in JHipster 5 Dec 16, 2017
@deepu105 deepu105 added the v5 label Dec 16, 2017
@jdubois
Copy link
Member

jdubois commented Dec 28, 2017

Hi everyone, as @JoffreyDcu went back to school, I don't think he will have time to split this PR into 3, as @deepu105 suggested (which would probably have been the best solution, indeed).
I'm going to merge this in a specific branch, and will start working on it - we need to upgrade to the latest Spring Boot 2.0 RC1 release, and use our new jhipster-dependencies project, so there's quite a lot of work.

@jdubois
Copy link
Member

jdubois commented Dec 28, 2017

Arrgh, according to GitHub there was no merge conflict, which was very surprising - and doing this manually shows a lot of conflicts, in fact!!!
I'm not sure this can be merged easily -> I'll first do an upgrade to Spring Boot 2 without the reactive option, as @deepu105 suggested. The main work here is to have the reactive option, I hope we can still get it without too much trouble.

@gmarziou
Copy link
Contributor

SB 2.0 release is planned for Feb, 20th that's quite a long time to keep this branch.

@jdubois
Copy link
Member

jdubois commented Dec 28, 2017

@gmarziou that's why I would do a branch with just the Spring Boot 2 upgrade, not the webflux stuff -> this is what is causing all the conflicts, as it's scattered all over the entites
The Spring Boot 2 upgrade alone shouldn't be a big change, so hopefully this can be maintained easily

@deepu105
Copy link
Member

closing since the work is now in another branch #7061

@deepu105 deepu105 closed this Jan 30, 2018
JHipster 5 automation moved this from In progress to Done Jan 30, 2018
@jdubois jdubois added this to the 5.0.0-beta.0 milestone Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
JHipster 5
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

8 participants