Merging changes for Third party dependencies upgrade #665
Conversation
| @@ -108,12 +108,6 @@ | |||
| <artifactId>cglib</artifactId> | |||
| <version>3.2.5</version> | |||
| </dependency> | |||
| <!-- Support for dynamic and type-safe JPA queries --> | |||
There was a problem hiding this comment.
Why was this query-dsl dependency removed from BOM? Does the new spring-boot parent already come with a more recent version of this dependency?
There was a problem hiding this comment.
I verified that 4.1.4 is used. Seems fine.
| @@ -51,6 +51,10 @@ | |||
| <artifactId>oasp4j-test</artifactId> | |||
| <scope>test</scope> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.springframework.boot</groupId> | |||
| <artifactId>spring-boot-starter</artifactId> | |||
There was a problem hiding this comment.
why do we need that for logging module? IMHO there is no need to couple this with spring.
Maybe just a test dependency and forgot to add the scope?
| @@ -48,20 +48,20 @@ public BinaryObjectEto saveBinaryObject(Blob data, BinaryObjectEto binaryObjectE | |||
| @Override | |||
| public void deleteBinaryObject(Long binaryObjectId) { | |||
|
|
|||
| this.binaryObjectRepository.delete(binaryObjectId); | |||
| this.binaryObjectRepository.deleteById(binaryObjectId); | |||
There was a problem hiding this comment.
Woah. So spring-data changed their API in an incompatible way? That is not so great to here. I will double check...
There was a problem hiding this comment.
Verified. Nothing we can do about it but good that we have these upgrades before we actually spread out spring-data support in devon.
| /** | ||
| * This method provide a new instance of {@code DelegatingPasswordEncoder}} | ||
| * | ||
| * @return the newly create {@code DelegatingPasswordEncoder}} |
There was a problem hiding this comment.
double occurance of closing curly brace ;)
| @@ -59,7 +59,7 @@ spring.batch.job.enabled=false | |||
|
|
|||
| # Flyway for Database Setup and Migrations | |||
| #if ($dbType == 'mariadb') | |||
| flyway.locations=classpath:db/migration,classpath:db/type/mysql | |||
| #spring.flyway.locations=classpath:db/migration,classpath:db/type/mysql | |||
There was a problem hiding this comment.
why was that commented out? Mistake or on purpose?
There was a problem hiding this comment.
Verified. Indeed a small mistake. Big change and aspect not covered by tests. I will fix this with a PR.
| @@ -29,10 +29,6 @@ | |||
| <version>${project.version}</version> | |||
| </dependency> | |||
| #end | |||
| <dependency> | |||
| <groupId>org.flywaydb</groupId> | |||
| <artifactId>flyway-core</artifactId> | |||
There was a problem hiding this comment.
was that redundant? Already in core I guess. Fine then. Thanks for cleanup.
There was a problem hiding this comment.
jep referenced in core and was temporary added as described in #664. All perfect.
| @@ -112,6 +108,7 @@ | |||
| <configuration> | |||
| <packagingExcludes>WEB-INF/classes/config/application.properties</packagingExcludes> | |||
| <warName>${project.artifactId}</warName> | |||
| <failOnMissingWebXml>false</failOnMissingWebXml> | |||
There was a problem hiding this comment.
I do not get why this had to be added? Did we downgrade the maven-war-plugin or what?
In such case it is always very helpful to add an XML comment with an hyperlink or a rationale.
There was a problem hiding this comment.
Effective version of maven-war-plugin now is 3.2.0 but that version was also used in the archetype as released before. I am quite sure there is a good reason for this change but I can not get it.
|
|
||
| } | ||
|
|
||
| @Override | ||
| public BinaryObjectEto findBinaryObject(Long binaryObjectId) { | ||
|
|
||
| return getBeanMapper().map(this.binaryObjectRepository.findOne(binaryObjectId), BinaryObjectEto.class); | ||
| return getBeanMapper().map(this.binaryObjectRepository.find(binaryObjectId), BinaryObjectEto.class); |
There was a problem hiding this comment.
As it seems the spring-data guys have renamed findOne to getOne. After things have been spread out to the wild these breaking API changes will not amuse the projects I assume. We should consider this for devonfw-forge/devcon#135
|
So I am now complete with the review and provided PR #668 for further improvements. |
#620