Using new features available in recent Java versions. #3053

chwoerz · 2023-07-05T13:56:46Z

I have applied the features available in new Java-versions by analyzing the files in IntelliJ.

You have read the Spring Data contribution guidelines.
You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
You submit test cases (unit or integration tests) that back your changes.
You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

pivotal-cla · 2023-07-05T13:56:51Z

@chwoerz Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-cla · 2023-07-05T13:58:33Z

@chwoerz Thank you for signing the Contributor License Agreement!

gregturn · 2023-07-05T19:04:37Z

...a-jpa/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java

+import jakarta.persistence.metamodel.Attribute.PersistentAttributeType;
+import jakarta.persistence.metamodel.ManagedType;
+import jakarta.persistence.metamodel.SingularAttribute;
+


I appreciate this PR as a way to update some of our coding constructions, taking advantage of Java 17 features.

There is certainly a lot of noise generating in the re-ordering of import statements. I can deal with this, but for future PR's, you may want to look into adopting https://github.com/spring-projects/spring-data-build/blob/main/etc/ide/README.md. We have settings configured for both Eclipse and IntelliJ so that we all have the same import statement/static import policies, and avoid generating too much churn just from that.

gregturn · 2023-07-05T19:07:03Z

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/EntityGraph.java

@@ -71,7 +70,7 @@
 	 * @author Thomas Darimont
 	 * @since 1.6
 	 */
-	public enum EntityGraphType {
+	enum EntityGraphType {



In general, don't alter visibility settings without first consulting with the team. We have a strict policy to avoid breaking changes and generally discuss changes in visibility before carrying them out.

Actually, this modifier has been redundant here because the enum is declared inside an annotation definition. So, this is not a breaking change.

gregturn · 2023-07-05T19:07:11Z

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/EntityGraph.java

@@ -95,7 +94,7 @@ public enum EntityGraphType {

 		private final String key;

-		private EntityGraphType(String value) {
+		EntityGraphType(String value) {
 			this.key = value;


In general, don't alter visibility settings without first consulting with the team. We have a strict policy to avoid breaking changes and generally discuss changes in visibility before carrying them out.

Actually, this modifier has been redundant here because the enum is declared inside an annotation definition. So, this is not a breaking change.

gregturn · 2023-07-05T19:10:20Z

...g-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryRenderer.java

-		ctx.joinSpecifier().forEach(joinSpecifierContext -> {
-			tokens.addAll(visit(joinSpecifierContext));
-		});
+		ctx.joinSpecifier().forEach(joinSpecifierContext -> tokens.addAll(visit(joinSpecifierContext)));



I am usually a fan of shortening things like this, but since this is an ANTLR Visitor implementation, I actually prefer to retain it the way it is. It makes it easy when grammar tweaks require me to add another tokens.addAll(...) inside that clause. And since this pattern is used A LOT in this class (and its counterpart), it becomes easier to maintain if I retain the pattern consistently everywhere.

gregturn · 2023-07-05T19:10:27Z

...g-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryRenderer.java

-			tokens.addAll(visit(caseWhenExpressionClauseContext));
-		});
+		ctx.caseWhenExpressionClause()
+				.forEach(caseWhenExpressionClauseContext -> tokens.addAll(visit(caseWhenExpressionClauseContext)));



I am usually a fan of shortening things like this, but since this is an ANTLR Visitor implementation, I actually prefer to retain it the way it is. It makes it easy when grammar tweaks require me to add another tokens.addAll(...) inside that clause. And since this pattern is used A LOT in this class (and its counterpart), it becomes easier to maintain if I retain the pattern consistently everywhere.

gregturn · 2023-07-05T19:15:09Z

...-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryRenderer.java

-		ctx.when_clause().forEach(whenClauseContext -> {
-			tokens.addAll(visit(whenClauseContext));
-		});
+		ctx.when_clause().forEach(whenClauseContext -> tokens.addAll(visit(whenClauseContext)));



I am usually a fan of shortening things like this, but since this is an ANTLR Visitor implementation, I actually prefer to retain it the way it is. It makes it easy when grammar tweaks require me to add another tokens.addAll(...) inside that clause. And since this pattern is used A LOT in this class (and its counterpart), it becomes easier to maintain if I retain the pattern consistently everywhere.

gregturn · 2023-07-05T19:15:13Z

...-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryRenderer.java

-		ctx.simple_when_clause().forEach(simpleWhenClauseContext -> {
-			tokens.addAll(visit(simpleWhenClauseContext));
-		});
+		ctx.simple_when_clause().forEach(simpleWhenClauseContext -> tokens.addAll(visit(simpleWhenClauseContext)));



I am usually a fan of shortening things like this, but since this is an ANTLR Visitor implementation, I actually prefer to retain it the way it is. It makes it easy when grammar tweaks require me to add another tokens.addAll(...) inside that clause. And since this pattern is used A LOT in this class (and its counterpart), it becomes easier to maintain if I retain the pattern consistently everywhere.

gregturn · 2023-07-05T19:23:07Z

spring-data-jpa/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java

 				.collect(StreamUtils.toUnmodifiableSet()));

 		this.jpaEmbeddables = Lazy.of(() -> metamodel.getEmbeddables().stream() //
-				.map(ManagedType::getJavaType)
-				.filter(it -> it != null)
+				.map(ManagedType::getJavaType).filter(Objects::nonNull)
 				.filter(it -> AnnotatedElementUtils.isAnnotated(it, Embeddable.class))


Please put a line comment at the end of the map operation, so that the filter stays on its own line.

gregturn · 2023-07-05T19:27:45Z

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/ItemId.java

 			return false;
-		return manufacturerId != null ? manufacturerId.equals(itemId.manufacturerId) : itemId.manufacturerId == null;
+		return Objects.equals(manufacturerId, itemId.manufacturerId);
 	}


We have a new template for equals at https://github.com/spring-projects/spring-data-build/blob/main/etc/ide/IntelliJ.equals-template.txt, which I'll run this by, to ensure we are compliant.

gregturn · 2023-07-05T19:27:50Z

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/ItemSiteId.java

 			return false;
-		return site != null ? site.equals(that.site) : that.site == null;
+		return Objects.equals(site, that.site);
 	}


We have a new template for equals at https://github.com/spring-projects/spring-data-build/blob/main/etc/ide/IntelliJ.equals-template.txt, which I'll run this by, to ensure we are compliant.

gregturn

In general, I need you to undo the changes in all the import statements. As it currently stands, the number of changes in this PR generates more work for us than the benefit it derives. If you reduce the volume of changes and focus on the REAL tweaks, then I'd be more open to processing in.

chwoerz added 2 commits July 5, 2023 15:23

used current java-version features and some cleanups

dcb80a2

Used project-formatter and added author-tag.

e1ac3db

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 5, 2023

gregturn reviewed Jul 5, 2023

View reviewed changes

gregturn added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 6, 2023

gregturn suggested changes Jul 6, 2023

View reviewed changes

mp911de force-pushed the main branch from 8219c96 to b47a340 Compare August 23, 2023 08:39

shin-mallang mentioned this pull request Aug 31, 2023

Apply code polishing to leverage various Java 17 structures. #3134

Closed

mp911de force-pushed the main branch from daebe31 to 1c37051 Compare August 12, 2024 07:10

mp911de force-pushed the main branch 2 times, most recently from 52ee55f to 61e6d36 Compare October 9, 2024 07:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using new features available in recent Java versions. #3053

Using new features available in recent Java versions. #3053

chwoerz commented Jul 5, 2023

pivotal-cla commented Jul 5, 2023

pivotal-cla commented Jul 5, 2023

gregturn Jul 5, 2023

gregturn Jul 5, 2023

AlexElin Mar 16, 2024

gregturn Jul 5, 2023

AlexElin Mar 16, 2024

gregturn Jul 5, 2023

gregturn Jul 5, 2023

gregturn Jul 5, 2023

gregturn Jul 5, 2023

gregturn Jul 5, 2023

gregturn Jul 5, 2023

gregturn Jul 5, 2023

gregturn left a comment

Using new features available in recent Java versions. #3053

Are you sure you want to change the base?

Using new features available in recent Java versions. #3053

Conversation

chwoerz commented Jul 5, 2023

pivotal-cla commented Jul 5, 2023

pivotal-cla commented Jul 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregturn left a comment

Choose a reason for hiding this comment