From c6603394efdc29f8bd99703e27dd3e90548899aa Mon Sep 17 00:00:00 2001 From: dk2k Date: Mon, 4 Oct 2021 17:26:26 +0300 Subject: [PATCH 1/4] changes based on the results of static code analysis by Findbugs --- .../convert/QueryByExamplePredicateBuilder.java | 2 +- .../jpa/mapping/JpaPersistentPropertyImpl.java | 1 + .../config/JpaRepositoryConfigExtension.java | 17 ++++++++++++++--- .../data/jpa/repository/query/StringQuery.java | 4 +--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java b/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java index 1b4e332501..be9b899d80 100644 --- a/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java +++ b/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java @@ -160,7 +160,7 @@ static List getPredicates(String path, CriteriaBuilder cb, Path fr PathNode node = currentNode.add(attribute.getName(), attributeValue); if (node.spansCycle()) { throw new InvalidDataAccessApiUsageException( - String.format("Path '%s' from root %s must not span a cyclic property reference!\r\n%s", currentPath, + String.format("Path '%s' from root %s must not span a cyclic property reference!%n%s", currentPath, ClassUtils.getShortName(probeType), node)); } diff --git a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java index a0ac87b813..4e9c06795a 100644 --- a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java +++ b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java @@ -333,6 +333,7 @@ private boolean detectUpdatability() { continue; } + // may cause NullPointerException if the returned value is null return (boolean) AnnotationUtils.getValue(annotation, "updatable"); } diff --git a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java index 7cdba8957d..0470b3bda3 100644 --- a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java +++ b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java @@ -16,6 +16,8 @@ package org.springframework.data.jpa.repository.config; import java.lang.annotation.Annotation; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -23,6 +25,7 @@ import java.util.Locale; import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import javax.persistence.Entity; import javax.persistence.MappedSuperclass; @@ -234,9 +237,17 @@ public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConf @Override protected ClassLoader getConfigurationInspectionClassLoader(ResourceLoader loader) { - ClassLoader classLoader = loader.getClassLoader(); - - return classLoader != null && LazyJvmAgent.isActive(loader.getClassLoader()) + AtomicReference classLoaderRef = new AtomicReference<>(); + AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Object run() { + // Classloaders should only be created inside doPrivileged block + classLoaderRef.set(loader.getClassLoader()); + return null; // nothing to return + } + }); + + return classLoaderRef.get() != null && LazyJvmAgent.isActive(loader.getClassLoader()) ? new InspectionClassLoader(loader.getClassLoader()) : loader.getClassLoader(); } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index 23c82cf6e8..dbac54211d 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -266,8 +266,6 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St String expression = spelExtractor.getParameter(parameterName == null ? parameterIndexString : parameterName); String replacement = null; - Assert.isTrue(parameterIndexString != null || parameterName != null, () -> String.format("We need either a name or an index! Offending query string: %s", query)); - expressionParameterIndex++; if ("".equals(parameterIndexString)) { @@ -293,7 +291,7 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St } else { checkAndRegister(new LikeParameterBinding(parameterName, likeType, expression), bindings); - replacement = expression != null ? ":" + parameterName : matcher.group(5); + replacement = ":" + parameterName; } break; From d7b75b97e9393c65adb96ebfcbe70d5529f7337f Mon Sep 17 00:00:00 2001 From: dk2k Date: Mon, 4 Oct 2021 18:11:46 +0300 Subject: [PATCH 2/4] added dummy space just to make a new commit --- .../springframework/data/jpa/repository/query/StringQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index dbac54211d..7b47395c46 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -264,7 +264,7 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St String typeSource = matcher.group(COMPARISION_TYPE_GROUP); String expression = spelExtractor.getParameter(parameterName == null ? parameterIndexString : parameterName); - String replacement = null; + String replacement = null; expressionParameterIndex++; if ("".equals(parameterIndexString)) { From 8e645acaba2582bc79679196d34a7756c41634a9 Mon Sep 17 00:00:00 2001 From: dk2k Date: Mon, 4 Oct 2021 18:12:50 +0300 Subject: [PATCH 3/4] removed dummy space just to make a new commit --- .../springframework/data/jpa/repository/query/StringQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index 7b47395c46..dbac54211d 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -264,7 +264,7 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St String typeSource = matcher.group(COMPARISION_TYPE_GROUP); String expression = spelExtractor.getParameter(parameterName == null ? parameterIndexString : parameterName); - String replacement = null; + String replacement = null; expressionParameterIndex++; if ("".equals(parameterIndexString)) { From b7ff99665b245858be9efbf26dda7fb58952a97e Mon Sep 17 00:00:00 2001 From: dk2k Date: Wed, 6 Oct 2021 13:01:33 +0300 Subject: [PATCH 4/4] removed AccessController usage --- .../config/JpaRepositoryConfigExtension.java | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java index 0470b3bda3..2dc3afbe29 100644 --- a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java +++ b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java @@ -16,8 +16,6 @@ package org.springframework.data.jpa.repository.config; import java.lang.annotation.Annotation; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -25,7 +23,6 @@ import java.util.Locale; import java.util.Optional; import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; import javax.persistence.Entity; import javax.persistence.MappedSuperclass; @@ -237,17 +234,9 @@ public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConf @Override protected ClassLoader getConfigurationInspectionClassLoader(ResourceLoader loader) { - AtomicReference classLoaderRef = new AtomicReference<>(); - AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Object run() { - // Classloaders should only be created inside doPrivileged block - classLoaderRef.set(loader.getClassLoader()); - return null; // nothing to return - } - }); - - return classLoaderRef.get() != null && LazyJvmAgent.isActive(loader.getClassLoader()) + ClassLoader classLoader = loader.getClassLoader(); + + return classLoader != null && LazyJvmAgent.isActive(loader.getClassLoader()) ? new InspectionClassLoader(loader.getClassLoader()) : loader.getClassLoader(); } @@ -261,7 +250,7 @@ public Object run() { * @return */ private static AbstractBeanDefinition getEntityManagerBeanDefinitionFor(RepositoryConfigurationSource config, - @Nullable Object source) { + @Nullable Object source) { BeanDefinitionBuilder builder = BeanDefinitionBuilder .rootBeanDefinition("org.springframework.orm.jpa.SharedEntityManagerCreator");