From a254e1adaec602a4afb03387ed955ca3d880dc14 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 8 Dec 2016 10:12:01 +0100 Subject: [PATCH] DATAREST-957 - Polish most critical Sonar warnings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed broken equals(…) in ProjectionDefinition. Switched to iterating over Map's entry set instead of the keys. Made UriAwareHttpServletRequest static. --- .../ProjectionDefinitionConfiguration.java | 53 +++++-------------- ...ctionDefinitionConfigurationUnitTests.java | 8 +-- .../webmvc/BasePathAwareHandlerMapping.java | 2 +- ...RepositoryPropertyReferenceController.java | 8 ++- ...ormationHandlerMethodArgumentResolver.java | 5 +- .../rest/webmvc/json/WrappedProperties.java | 7 +-- 6 files changed, 30 insertions(+), 53 deletions(-) diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java index a13665062..321258ebd 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java @@ -15,6 +15,11 @@ */ package org.springframework.data.rest.core.config; +import lombok.AccessLevel; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import lombok.Value; + import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -118,7 +123,7 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions Assert.notEmpty(sourceTypes, "Source types must not be null!"); for (Class sourceType : sourceTypes) { - this.projectionDefinitions.add(new ProjectionDefinition(sourceType, projectionType, name)); + this.projectionDefinitions.add(ProjectionDefinition.of(sourceType, projectionType, name)); } return this; @@ -189,10 +194,12 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions * * @author Oliver Gierke */ + @Value + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) static final class ProjectionDefinition { - private final Class sourceType, targetType; - private final String name; + private final @NonNull Class sourceType, targetType; + private final @NonNull String name; /** * Creates a new {@link ProjectionDefinitionKey} for the given source type and name; @@ -201,47 +208,11 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions * @param targetType must not be {@literal null}. * @param name must not be {@literal null} or empty. */ - public ProjectionDefinition(Class sourceType, Class targetType, String name) { + static ProjectionDefinition of(Class sourceType, Class targetType, String name) { - Assert.notNull(sourceType, "Source type must not be null!"); - Assert.notNull(targetType, "Target type must not be null!"); Assert.hasText(name, "Name must not be null or empty!"); - this.sourceType = sourceType; - this.targetType = targetType; - this.name = name; - } - - /* - * (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ - @Override - public boolean equals(Object obj) { - - if (!(obj instanceof ProjectionDefinition)) { - return false; - } - - ProjectionDefinition that = (ProjectionDefinition) obj; - return this.name.equals(that.name) && this.sourceType.equals(that.sourceType) - && this.sourceType.equals(that.sourceType); - } - - /* - * (non-Javadoc) - * @see java.lang.Object#hashCode() - */ - @Override - public int hashCode() { - - int result = 31; - - result += name.hashCode(); - result += sourceType.hashCode(); - result += targetType.hashCode(); - - return result; + return new ProjectionDefinition(sourceType, targetType, name); } } } diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java index a0cc7833c..6c9615073 100644 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java @@ -121,10 +121,10 @@ public class ProjectionDefinitionConfigurationUnitTests { @Test public void definitionEquals() { - ProjectionDefinition objectName = new ProjectionDefinition(Object.class, Object.class, "name"); - ProjectionDefinition sameObjectName = new ProjectionDefinition(Object.class, Object.class, "name"); - ProjectionDefinition stringName = new ProjectionDefinition(String.class, Object.class, "name"); - ProjectionDefinition objectOtherNameKey = new ProjectionDefinition(Object.class, Object.class, "otherName"); + ProjectionDefinition objectName = ProjectionDefinition.of(Object.class, Object.class, "name"); + ProjectionDefinition sameObjectName = ProjectionDefinition.of(Object.class, Object.class, "name"); + ProjectionDefinition stringName = ProjectionDefinition.of(String.class, Object.class, "name"); + ProjectionDefinition objectOtherNameKey = ProjectionDefinition.of(Object.class, Object.class, "otherName"); assertThat(objectName, is(objectName)); assertThat(objectName, is(sameObjectName)); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java index c80aa61cb..1318c7030 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java @@ -190,7 +190,7 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { super.afterPropertiesSet(); } - private class UriAwareHttpServletRequest implements HttpServletRequest { + private static class UriAwareHttpServletRequest implements HttpServletRequest { private final ServletContext context; private final String path; diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java index d87ebb173..d5a879d29 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java @@ -382,10 +382,13 @@ class RepositoryPropertyReferenceController extends AbstractRepositoryRestContro } } } else if (prop.property.isMap()) { + Map m = (Map) prop.propertyValue; - Iterator itr = m.keySet().iterator(); + Iterator> itr = m.entrySet().iterator(); + while (itr.hasNext()) { - Object key = itr.next(); + + Object key = itr.next().getKey(); IdentifierAccessor accessor = prop.entity.getIdentifierAccessor(m.get(key)); String s = accessor.getIdentifier().toString(); @@ -394,6 +397,7 @@ class RepositoryPropertyReferenceController extends AbstractRepositoryRestContro itr.remove(); } } + } else { prop.accessor.setProperty(prop.property, null); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java index 762b76669..366d3134b 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java @@ -17,6 +17,7 @@ package org.springframework.data.rest.webmvc.config; import java.util.Arrays; import java.util.Map; +import java.util.Map.Entry; import org.springframework.core.MethodParameter; import org.springframework.data.querydsl.QueryDslPredicateExecutor; @@ -103,8 +104,8 @@ class QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver MultiValueMap result = new LinkedMultiValueMap(); - for (String key : source.keySet()) { - result.put(key, Arrays.asList(source.get(key))); + for (Entry entry : source.entrySet()) { + result.put(entry.getKey(), Arrays.asList(entry.getValue())); } return result; diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java index 664213863..efa80ea8c 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -166,14 +167,14 @@ class WrappedProperties { Map>> nestedProperties = findUnwrappedPropertyPaths( annotatedMember.getRawType(), propertyNameTransformer, true); - for (String key : nestedProperties.keySet()) { + for (Entry>> entry : nestedProperties.entrySet()) { List> persistentProperties = new ArrayList>(); persistentProperties.add(persistentProperty); - persistentProperties.addAll(nestedProperties.get(key)); + persistentProperties.addAll(entry.getValue()); - mapping.put(key, persistentProperties); + mapping.put(entry.getKey(), persistentProperties); } return mapping;