From eab7e70365df5b20d203ed31be8e2a95557a149c Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 12 Oct 2017 11:35:31 +0200 Subject: [PATCH] SONAR-9943 add InternalPropertiesDao#selectByKeys --- .../db/property/InternalPropertiesDao.java | 73 ++++++++++- .../db/property/InternalPropertiesMapper.java | 5 +- .../db/property/InternalPropertiesMapper.xml | 6 +- .../property/InternalPropertiesDaoTest.java | 121 ++++++++++++++++++ 4 files changed, 198 insertions(+), 7 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java index e9e500bc3e5..b2bd5fe16c0 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java @@ -19,7 +19,18 @@ */ package org.sonar.db.property; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.utils.System2; import org.sonar.api.utils.log.Loggers; @@ -27,6 +38,8 @@ import org.sonar.db.Dao; import org.sonar.db.DbSession; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.util.Collections.singletonList; public class InternalPropertiesDao implements Dao { @@ -76,6 +89,50 @@ public class InternalPropertiesDao implements Dao { mapper.insertAsEmpty(key, system2.now()); } + /** + * @return a Map with an {link Optional} for each String in {@code keys}. + */ + public Map> selectByKeys(DbSession dbSession, @Nullable Set keys) { + if (keys == null || keys.isEmpty()) { + return Collections.emptyMap(); + } + if (keys.size() == 1) { + String key = keys.iterator().next(); + return ImmutableMap.of(key, selectByKey(dbSession, key)); + } + keys.forEach(InternalPropertiesDao::checkKey); + + InternalPropertiesMapper mapper = getMapper(dbSession); + List res = mapper.selectAsText(ImmutableList.copyOf(keys)); + Map> builder = new HashMap<>(keys.size()); + res.forEach(internalPropertyDto -> { + String key = internalPropertyDto.getKey(); + if (internalPropertyDto.isEmpty()) { + builder.put(key, OPTIONAL_OF_EMPTY_STRING); + } + if (internalPropertyDto.getValue() != null) { + builder.put(key, Optional.of(internalPropertyDto.getValue())); + } + }); + // return Optional.empty() for all keys without a DB entry + Sets.difference(keys, res.stream().map(InternalPropertyDto::getKey).collect(Collectors.toSet())) + .forEach(key -> builder.put(key, Optional.empty())); + // keys for which there isn't a text or empty value found yet + List keyWithClobValue = ImmutableList.copyOf(Sets.difference(keys, builder.keySet())); + if (keyWithClobValue.isEmpty()) { + return ImmutableMap.copyOf(builder); + } + + // retrieve properties with a clob value + res = mapper.selectAsClob(keyWithClobValue); + res.forEach(internalPropertyDto -> builder.put(internalPropertyDto.getKey(), Optional.of(internalPropertyDto.getValue()))); + + // return Optional.empty() for all key with a DB entry which neither has text value, nor is empty nor has clob value + Sets.difference(ImmutableSet.copyOf(keyWithClobValue), builder.keySet()).forEach(key -> builder.put(key, Optional.empty())); + + return ImmutableMap.copyOf(builder); + } + /** * No streaming of value */ @@ -83,7 +140,7 @@ public class InternalPropertiesDao implements Dao { checkKey(key); InternalPropertiesMapper mapper = getMapper(dbSession); - InternalPropertyDto res = mapper.selectAsText(key); + InternalPropertyDto res = enforceSingleElement(key, mapper.selectAsText(singletonList(key))); if (res == null) { return Optional.empty(); } @@ -93,16 +150,26 @@ public class InternalPropertiesDao implements Dao { if (res.getValue() != null) { return Optional.of(res.getValue()); } - res = mapper.selectAsClob(key); + res = enforceSingleElement(key, mapper.selectAsClob(singletonList(key))); if (res == null) { Loggers.get(InternalPropertiesDao.class) .debug("Internal property {} has been found in db but has neither text value nor is empty. " + - "Still we couldn't be retrieved with clob value. Ignoring the property.", key); + "Still it couldn't be retrieved with clob value. Ignoring the property.", key); return Optional.empty(); } return Optional.of(res.getValue()); } + @CheckForNull + private static InternalPropertyDto enforceSingleElement(String key, List rows) { + if (rows.isEmpty()) { + return null; + } + int size = rows.size(); + checkState(size <= 1, "%s rows retrieved for single property %s", size, key); + return rows.iterator().next(); + } + private static void checkKey(@Nullable String key) { checkArgument(key != null && !key.isEmpty(), "key can't be null nor empty"); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java index be1e5b122b4..e329b1fc361 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java @@ -19,12 +19,13 @@ */ package org.sonar.db.property; +import java.util.List; import org.apache.ibatis.annotations.Param; public interface InternalPropertiesMapper { - InternalPropertyDto selectAsText(@Param("key") String key); + List selectAsText(@Param("keys") List key); - InternalPropertyDto selectAsClob(@Param("key") String key); + List selectAsClob(@Param("keys") List key); void insertAsEmpty(@Param("key") String key, @Param("createdAt") long createdAt); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml index 7c0ddfcd318..a606d9148a6 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml @@ -5,24 +5,26 @@ diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java index 53e74704c57..babea6ffa75 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java @@ -19,8 +19,18 @@ */ package org.sonar.db.property; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.Random; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.assertj.core.api.AbstractAssert; @@ -34,20 +44,26 @@ import org.sonar.db.DbTester; import static java.lang.Boolean.FALSE; import static java.lang.Boolean.TRUE; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class InternalPropertiesDaoTest { private static final String EMPTY_STRING = ""; private static final String A_KEY = "a_key"; + private static final String ANOTHER_KEY = "another_key"; private static final String VALUE_1 = "one"; private static final String VALUE_2 = "two"; private static final long DATE_1 = 1_500_000_000_000L; private static final long DATE_2 = 1_600_000_000_000L; private static final String VALUE_SMALL = "some small value"; + private static final String OTHER_VALUE_SMALL = "other small value"; private static final String VALUE_SIZE_4000 = String.format("%1$4000.4000s", "*"); private static final String VALUE_SIZE_4001 = VALUE_SIZE_4000 + "P"; + private static final String OTHER_VALUE_SIZE_4001 = VALUE_SIZE_4000 + "D"; private System2 system2 = mock(System2.class); @@ -275,6 +291,111 @@ public class InternalPropertiesDaoTest { assertThat(underTest.selectByKey(dbSession, A_KEY)).contains(VALUE_SIZE_4001); } + @Test + public void selectByKeys_returns_empty_map_if_keys_is_null() { + assertThat(underTest.selectByKeys(dbSession, null)).isEmpty(); + } + + @Test + public void selectByKeys_returns_empty_map_if_keys_is_empty() { + assertThat(underTest.selectByKeys(dbSession, Collections.emptySet())).isEmpty(); + } + + @Test + public void selectByKeys_throws_IAE_when_keys_contains_null() { + Random random = new Random(); + Set keysIncludingANull = Stream.of( + IntStream.range(0, random.nextInt(10)).mapToObj(i -> "b_" + i), + Stream.of((String) null), + IntStream.range(0, random.nextInt(10)).mapToObj(i -> "a_" + i)) + .flatMap(s -> s) + .collect(Collectors.toSet()); + + expectKeyNullOrEmptyIAE(); + + underTest.selectByKeys(dbSession, keysIncludingANull); + } + + @Test + public void selectByKeys_throws_IAE_when_keys_contains_empty_string() { + Random random = new Random(); + Set keysIncludingAnEmptyString = Stream.of( + IntStream.range(0, random.nextInt(10)).mapToObj(i -> "b_" + i), + Stream.of(""), + IntStream.range(0, random.nextInt(10)).mapToObj(i -> "a_" + i)) + .flatMap(s -> s) + .collect(Collectors.toSet()); + + expectKeyNullOrEmptyIAE(); + + underTest.selectByKeys(dbSession, keysIncludingAnEmptyString); + } + + @Test + public void selectByKeys_returns_empty_optional_when_property_does_not_exist_in_DB() { + assertThat(underTest.selectByKeys(dbSession, ImmutableSet.of(A_KEY, ANOTHER_KEY))) + .containsOnly( + entry(A_KEY, Optional.empty()), + entry(ANOTHER_KEY, Optional.empty())); + } + + @Test + public void selectByKeys_returns_empty_string_when_property_is_empty_in_DB() { + underTest.saveAsEmpty(dbSession, A_KEY); + underTest.saveAsEmpty(dbSession, ANOTHER_KEY); + + assertThat(underTest.selectByKeys(dbSession, ImmutableSet.of(A_KEY, ANOTHER_KEY))) + .containsOnly( + entry(A_KEY, Optional.of("")), + entry(ANOTHER_KEY, Optional.of(""))); + } + + @Test + public void selectByKeys_returns_value_when_property_has_value_stored_in_varchar() { + underTest.save(dbSession, A_KEY, VALUE_SMALL); + underTest.save(dbSession, ANOTHER_KEY, OTHER_VALUE_SMALL); + + assertThat(underTest.selectByKeys(dbSession, ImmutableSet.of(A_KEY, ANOTHER_KEY))) + .containsOnly( + entry(A_KEY, Optional.of(VALUE_SMALL)), + entry(ANOTHER_KEY, Optional.of(OTHER_VALUE_SMALL))); + } + + @Test + public void selectByKeys_returns_values_when_properties_has_value_stored_in_clob() { + underTest.save(dbSession, A_KEY, VALUE_SIZE_4001); + underTest.save(dbSession, ANOTHER_KEY, OTHER_VALUE_SIZE_4001); + + assertThat(underTest.selectByKeys(dbSession, ImmutableSet.of(A_KEY, ANOTHER_KEY))) + .containsOnly( + entry(A_KEY, Optional.of(VALUE_SIZE_4001)), + entry(ANOTHER_KEY, Optional.of(OTHER_VALUE_SIZE_4001))); + } + + @Test + public void selectByKeys_queries_only_clob_properties_with_clob_SQL_query() { + underTest.saveAsEmpty(dbSession, A_KEY); + underTest.save(dbSession, "key2", VALUE_SMALL); + underTest.save(dbSession, "key3", VALUE_SIZE_4001); + Set keys = ImmutableSet.of(A_KEY, "key2", "key3", "non_existent_key"); + List allInternalPropertyDtos = dbSession.getMapper(InternalPropertiesMapper.class).selectAsText(ImmutableList.copyOf(keys)); + List clobPropertyDtos = dbSession.getMapper(InternalPropertiesMapper.class).selectAsClob(ImmutableList.of("key3")); + + InternalPropertiesMapper mapperMock = mock(InternalPropertiesMapper.class); + DbSession dbSessionMock = mock(DbSession.class); + when(dbSessionMock.getMapper(InternalPropertiesMapper.class)).thenReturn(mapperMock); + when(mapperMock.selectAsText(ImmutableList.copyOf(keys))) + .thenReturn(allInternalPropertyDtos); + when(mapperMock.selectAsClob(ImmutableList.of("key3"))) + .thenReturn(clobPropertyDtos); + + underTest.selectByKeys(dbSessionMock, keys); + + verify(mapperMock).selectAsText(ImmutableList.copyOf(keys)); + verify(mapperMock).selectAsClob(ImmutableList.of("key3")); + verifyNoMoreInteractions(mapperMock); + } + private void expectKeyNullOrEmptyIAE() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("key can't be null nor empty"); -- 2.39.5