From 3fa574c4b6a111a8c7fa830b0cefb7a27ae9e681 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Thu, 7 Apr 2011 02:52:04 +0400 Subject: [PATCH] SONAR-2225 JpaDatabaseSession: improve exception message in case of NonUniqueResultException --- .../sonar/jpa/session/JpaDatabaseSession.java | 54 ++++++++++++--- .../jpa/session/JpaDatabaseSessionTest.java | 68 +++++++++++++++++++ 2 files changed, 113 insertions(+), 9 deletions(-) create mode 100644 sonar-core/src/test/java/org/sonar/jpa/session/JpaDatabaseSessionTest.java diff --git a/sonar-core/src/main/java/org/sonar/jpa/session/JpaDatabaseSession.java b/sonar-core/src/main/java/org/sonar/jpa/session/JpaDatabaseSession.java index b407bacfe2a..6c87056f278 100644 --- a/sonar-core/src/main/java/org/sonar/jpa/session/JpaDatabaseSession.java +++ b/sonar-core/src/main/java/org/sonar/jpa/session/JpaDatabaseSession.java @@ -23,14 +23,12 @@ import org.apache.commons.lang.StringUtils; import org.slf4j.LoggerFactory; import org.sonar.api.database.DatabaseSession; +import java.util.*; + import javax.persistence.EntityManager; -import javax.persistence.NoResultException; import javax.persistence.NonUniqueResultException; +import javax.persistence.PersistenceException; import javax.persistence.Query; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; public class JpaDatabaseSession extends DatabaseSession { @@ -43,6 +41,10 @@ public class JpaDatabaseSession extends DatabaseSession { this.connector = connector; } + /** + * Note that usage of this method is discouraged, because it allows to construct and execute queries without additional exception handling, + * which done in methods of this class. + */ public EntityManager getEntityManager() { return entityManager; } @@ -143,17 +145,43 @@ public class JpaDatabaseSession extends DatabaseSession { } } - + /** + * Note that not recommended to directly execute {@link Query#getSingleResult()}, because it will bypass exception handling, + * which done in {@link #getSingleResult(Query, Object)}. + */ public Query createQuery(String hql) { startTransaction(); return entityManager.createQuery(hql); } + /** + * @return the result or defaultValue, if not found + * @throws NonUniqueResultException if more than one result + */ public T getSingleResult(Query query, T defaultValue) { - try { - return (T) query.getSingleResult(); - } catch (NoResultException ex) { + /* + * See http://jira.codehaus.org/browse/SONAR-2225 + * By default Hibernate throws NonUniqueResultException without meaningful information about context, + * so we improve it here by adding all results in error message. + * Note that in some rare situations we can receive too many results, which may lead to OOME, + * but actually it will mean that database is corrupted as we don't expect more than one result + * and in fact org.hibernate.ejb.QueryImpl#getSingleResult() anyway does loading of several results under the hood. + */ + List result = query.getResultList(); + + if (result.size() == 1) { + return result.get(0); + + } else if (result.isEmpty()) { return defaultValue; + + } else { + Set uniqueResult = new HashSet(result); + if (uniqueResult.size() > 1) { + throw new NonUniqueResultException("Expected single result, but got : " + result.toString()); + } else { + return uniqueResult.iterator().next(); + } } } @@ -162,11 +190,19 @@ public class JpaDatabaseSession extends DatabaseSession { return getEntityManager().find(entityClass, id); } + /** + * @return the result or null, if not found + * @throws NonUniqueResultException if more than one result + */ public T getSingleResult(Class entityClass, Object... criterias) { try { return getSingleResult(getQueryForCriterias(entityClass, true, criterias), (T) null); } catch (NonUniqueResultException ex) { + /* + * TODO Log and throw is anti-pattern ( see http://today.java.net/article/2006/04/04/exception-handling-antipatterns#logAndThrow ), + * but NonUniqueResultException doesn't have a constructor with cause + */ LoggerFactory.getLogger(JpaDatabaseSession.class).warn("NonUniqueResultException on entity {} with criterias : {}", entityClass.getSimpleName(), StringUtils.join(criterias, ",")); throw ex; diff --git a/sonar-core/src/test/java/org/sonar/jpa/session/JpaDatabaseSessionTest.java b/sonar-core/src/test/java/org/sonar/jpa/session/JpaDatabaseSessionTest.java new file mode 100644 index 00000000000..d1314ea4316 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/jpa/session/JpaDatabaseSessionTest.java @@ -0,0 +1,68 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2011 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.jpa.session; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; + +import javax.persistence.NonUniqueResultException; +import javax.persistence.Query; + +public class JpaDatabaseSessionTest { + + private JpaDatabaseSession session; + + @Before + public void setUp() { + session = new JpaDatabaseSession(null); + } + + @Test(expected = NonUniqueResultException.class) + public void shouldThrowNonUniqueResultException() { + Query query = mock(Query.class); + when(query.getResultList()).thenReturn(Arrays.asList("foo", "bar")); + session.getSingleResult(query, null); + } + + @Test + public void shouldReturnSingleResult() { + Query query = mock(Query.class); + + when(query.getResultList()).thenReturn(Arrays.asList("foo", "foo"), Arrays.asList("bar")); + assertThat(session.getSingleResult(query, "default"), is("foo")); + assertThat(session.getSingleResult(query, "default"), is("bar")); + } + + @Test + public void shouldReturnDefaultValue() { + Query query = mock(Query.class); + when(query.getResultList()).thenReturn(Collections.emptyList()); + assertThat(session.getSingleResult(query, "default"), is("default")); + } + +} -- 2.39.5