From 4c0a3bcd818d707cd04960f6decdd86e1c4fa7c4 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 26 Mar 2018 16:48:09 +0200 Subject: [PATCH] DATAJPA-1301 - Improved null handling for TupleBackedMap. TupleBackedMap now properly distinguishes between tuple elements with a null value and non-existing elements. Original pull request: #262. --- .../repository/query/AbstractJpaQuery.java | 42 ++++++++++++++++--- ...lipseLinkNamespaceUserRepositoryTests.java | 7 ++++ .../jpa/repository/UserRepositoryTests.java | 22 ++++++++++ .../jpa/repository/sample/UserRepository.java | 5 +++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 65f0d3729..31e0dee8c 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -317,6 +317,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { */ private static class TupleBackedMap implements Map { + public static final String UNMODIFIABLE_MESSAGE = "A TupleBackedMap cannot be modified."; private final Tuple tuple; TupleBackedMap(Tuple tuple) { @@ -333,9 +334,22 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { return tuple.getElements().isEmpty(); } + /** + * If the key is not a {@code String} or not a key of the backing {@link Tuple} this returns {@code false}. + * Otherwise this returns {@code true} even when the value from the backing {@code Tuple} is {@code null}. + * + * @param key the key for which to get the value from the map. + * @return wether the key is an element of the backing tuple. + */ @Override public boolean containsKey(Object key) { - return key instanceof String && tuple.get((String) key) != null; + + try { + tuple.get((String) key); + return true; + } catch (IllegalArgumentException e) { + return false; + } } @Override @@ -343,29 +357,45 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { return Arrays.asList(tuple.toArray()).contains(value); } + /** + * If the key is not a {@code String} or not a key of the backing {@link Tuple} this returns {@code null}. + * Otherwise the value from the backing {@code Tuple} is returned, which also might be {@code null}. + * + * @param key the key for which to get the value from the map. + * @return the value of the backing {@link Tuple} for that key or {@code null}. + */ @Override public Object get(Object key) { - return key instanceof String ? tuple.get((String) key) : null; + + if (!(key instanceof String)) { + return null; + } + + try { + return tuple.get((String) key); + } catch (IllegalArgumentException e) { + return null; + } } @Override public Object put(String key, Object value) { - throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + throw new UnsupportedOperationException(UNMODIFIABLE_MESSAGE); } @Override public Object remove(Object key) { - throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + throw new UnsupportedOperationException(UNMODIFIABLE_MESSAGE); } @Override public void putAll(Map m) { - throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + throw new UnsupportedOperationException(UNMODIFIABLE_MESSAGE); } @Override public void clear() { - throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + throw new UnsupportedOperationException(UNMODIFIABLE_MESSAGE); } @Override diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index 8c56c78ff..edc7774f0 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -99,6 +99,13 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi @Override public void supportsProjectionsWithNativeQueriesAndCamelCaseProperty() {} + /** + * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=525319 is fixed. + */ + @Override + @Test // DATAJPA-1301 + public void returnsNullValueInMap() {} + /** * TODO: Remove, once https://bugs.eclipse.org/bugs/show_bug.cgi?id=289141 is fixed. */ diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index badf9a166..98e5d2715 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -31,6 +31,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -2235,6 +2236,27 @@ public class UserRepositoryTests { assertThat(element.getLastname(), is("Arrasz")); } + @Test // DATAJPA-1301 + public void returnsNullValueInMap() { + + Assume + .assumeTrue(getHibernateVersion().isGreaterThanOrEqualTo(HIBERNATE_VERSION_SUPPORTING_TUPLE_ON_NATIVE_QUERIES)); + + firstUser.setLastname(null); + flushTestUsers(); + + Map map = repository.findMapWithNullValues(); + + assertThat(map.keySet(), contains("firstname", "lastname")); + assertThat(map.containsKey("firstname"), is(true)); + assertThat(map.containsKey("lastname"), is(true)); + + assertThat(map.get("firstname"), is((Object) "Oliver")); + assertThat(map.get("lastname"), is(nullValue())); + assertThat(map.get("non-existent"), is(nullValue())); + assertThat(map.get(new Object()), is(nullValue())); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index 5c7e57579..6ef51f702 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -18,6 +18,7 @@ package org.springframework.data.jpa.repository.sample; import java.util.Collection; import java.util.Date; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Stream; @@ -489,6 +490,10 @@ public interface UserRepository // DATAJPA-1273 List findByNamedQueryWithAliasInInvertedOrder(); + // DATAJPA-1301 + @Query("select firstname as firstname, lastname as lastname from User u where u.firstname = 'Oliver'") + Map findMapWithNullValues(); + interface RolesAndFirstname { String getFirstname();