diff options
18 files changed, 290 insertions, 168 deletions
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFilter.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFilter.java index db2b2b9fbf7..f2269b81a3e 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFilter.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFilter.java @@ -20,25 +20,10 @@ package org.sonar.core.issue; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Splitter; -import com.google.common.collect.Lists; -import org.apache.commons.lang.StringUtils; - import java.util.Date; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - -import static com.google.common.collect.Lists.newArrayList; -import static com.google.common.collect.Maps.newHashMap; public class DefaultIssueFilter { - public static final String SEPARATOR = "|"; - public static final String KEY_VALUE_SEPARATOR = "="; - public static final String LIST_SEPARATOR = ","; - private Long id; private String name; private String user; @@ -60,10 +45,6 @@ public class DefaultIssueFilter { return issueFilter; } - public DefaultIssueFilter(Map<String, Object> mapData) { - setData(mapData); - } - public Long id() { return id; } @@ -136,80 +117,4 @@ public class DefaultIssueFilter { return this; } - public final DefaultIssueFilter setData(Map<String, Object> mapData) { - this.data = mapAsdata(mapData); - return this; - } - - /** - * Used by ui - */ - public Map<String, Object> dataAsMap(){ - return dataAsMap(data); - } - - @VisibleForTesting - final Map<String, Object> dataAsMap(String data) { - Map<String, Object> map = newHashMap(); - - Iterable<String> keyValues = Splitter.on(DefaultIssueFilter.SEPARATOR).split(data); - for (String keyValue : keyValues) { - String[] keyValueSplit = StringUtils.split(keyValue, DefaultIssueFilter.KEY_VALUE_SEPARATOR); - if (keyValueSplit.length == 2) { - String key = keyValueSplit[0]; - String value = keyValueSplit[1]; - String[] listValues = StringUtils.split(value, DefaultIssueFilter.LIST_SEPARATOR); - if (listValues.length > 1) { - map.put(key, newArrayList(listValues)); - } else { - map.put(key, value); - } - } - } - return map; - } - - @VisibleForTesting - final String mapAsdata(Map<String, Object> map) { - StringBuilder stringBuilder = new StringBuilder(); - - for (Map.Entry<String, Object> entries : map.entrySet()){ - String key = entries.getKey(); - Object value = entries.getValue(); - if (value != null) { - List valuesList = newArrayList(); - if (value instanceof List) { - // assume that it contains only strings - valuesList = (List) value; - } else if (value instanceof CharSequence) { - valuesList = Lists.newArrayList(Splitter.on(',').omitEmptyStrings().split((CharSequence) value)); - } else { - stringBuilder.append(key); - stringBuilder.append(DefaultIssueFilter.KEY_VALUE_SEPARATOR); - stringBuilder.append(value); - stringBuilder.append(DefaultIssueFilter.SEPARATOR); - } - if (!valuesList.isEmpty()) { - stringBuilder.append(key); - stringBuilder.append(DefaultIssueFilter.KEY_VALUE_SEPARATOR); - for (Iterator<Object> valueListIter = valuesList.iterator(); valueListIter.hasNext();) { - Object valueList = valueListIter.next(); - stringBuilder.append(valueList); - if (valueListIter.hasNext()) { - stringBuilder.append(DefaultIssueFilter.LIST_SEPARATOR); - } - } - stringBuilder.append(DefaultIssueFilter.SEPARATOR); - } - } - } - - if (stringBuilder.length() > 0) { - // Delete useless last separator character - stringBuilder.deleteCharAt(stringBuilder.length() - 1); - } - - return stringBuilder.toString(); - } - } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueFilterSerializer.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueFilterSerializer.java new file mode 100644 index 00000000000..3a1f5d20c9c --- /dev/null +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueFilterSerializer.java @@ -0,0 +1,107 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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 this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.core.issue; + +import com.google.common.base.Splitter; +import com.google.common.collect.Lists; +import org.apache.commons.lang.StringUtils; +import org.sonar.api.ServerComponent; + +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; + +public class IssueFilterSerializer implements ServerComponent { + + public static final String SEPARATOR = "|"; + public static final String KEY_VALUE_SEPARATOR = "="; + public static final String LIST_SEPARATOR = ","; + + public IssueFilterSerializer() { + + } + + public String serialize(Map<String, Object> map) { + StringBuilder stringBuilder = new StringBuilder(); + + for (Map.Entry<String, Object> entries : map.entrySet()) { + String key = entries.getKey(); + Object value = entries.getValue(); + if (value != null) { + List valuesList = newArrayList(); + if (value instanceof List) { + // assume that it contains only strings + valuesList = (List) value; + } else if (value instanceof CharSequence) { + valuesList = Lists.newArrayList(Splitter.on(',').omitEmptyStrings().split((CharSequence) value)); + } else { + stringBuilder.append(key); + stringBuilder.append(IssueFilterSerializer.KEY_VALUE_SEPARATOR); + stringBuilder.append(value); + stringBuilder.append(IssueFilterSerializer.SEPARATOR); + } + if (!valuesList.isEmpty()) { + stringBuilder.append(key); + stringBuilder.append(IssueFilterSerializer.KEY_VALUE_SEPARATOR); + for (Iterator<Object> valueListIter = valuesList.iterator(); valueListIter.hasNext(); ) { + Object valueList = valueListIter.next(); + stringBuilder.append(valueList); + if (valueListIter.hasNext()) { + stringBuilder.append(IssueFilterSerializer.LIST_SEPARATOR); + } + } + stringBuilder.append(IssueFilterSerializer.SEPARATOR); + } + } + } + + if (stringBuilder.length() > 0) { + // Delete useless last separator character + stringBuilder.deleteCharAt(stringBuilder.length() - 1); + } + + return stringBuilder.toString(); + } + + public Map<String, Object> deserialize(String data) { + Map<String, Object> map = newHashMap(); + + Iterable<String> keyValues = Splitter.on(IssueFilterSerializer.SEPARATOR).split(data); + for (String keyValue : keyValues) { + String[] keyValueSplit = StringUtils.split(keyValue, IssueFilterSerializer.KEY_VALUE_SEPARATOR); + if (keyValueSplit.length == 2) { + String key = keyValueSplit[0]; + String value = keyValueSplit[1]; + String[] listValues = StringUtils.split(value, IssueFilterSerializer.LIST_SEPARATOR); + if (listValues.length > 1) { + map.put(key, newArrayList(listValues)); + } else { + map.put(key, value); + } + } + } + return map; + } + +} diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java index b4ffcaeb880..572d23e41ed 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java @@ -80,10 +80,19 @@ public class IssueFilterDao implements BatchComponent, ServerComponent { } } - public List<IssueFilterDto> selectSharedForUser(String user) { + public List<IssueFilterDto> selectSharedWithoutUserFilters(String user) { SqlSession session = mybatis.openSession(); try { - return getMapper(session).selectSharedForUser(user); + return getMapper(session).selectSharedWithoutUserFilters(user); + } finally { + MyBatis.closeQuietly(session); + } + } + + public IssueFilterDto selectSharedWithoutUserFiltersByName(String name, String user, @Nullable Long existingId) { + SqlSession session = mybatis.openSession(); + try { + return getMapper(session).selectSharedWithoutUserFiltersByName(name, user, existingId); } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java index 53cf2b936eb..eb634af22b6 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java @@ -41,7 +41,10 @@ public interface IssueFilterMapper { List<IssueFilterDto> selectByUserWithOnlyFavoriteFilters(String user); - List<IssueFilterDto> selectSharedForUser(String user); + List<IssueFilterDto> selectSharedWithoutUserFilters(String user); + + @CheckForNull + IssueFilterDto selectSharedWithoutUserFiltersByName(@Param("name") String name, @Param("userLogin") String userLogin, @Nullable @Param("existingId") Long existingId); void insert(IssueFilterDto filter); diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml index a31b00273c5..a85abad3bbe 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml @@ -52,12 +52,25 @@ </where> </select> - <select id="selectSharedForUser" parameterType="String" resultType="IssueFilter"> + <select id="selectSharedWithoutUserFilters" parameterType="String" resultType="IssueFilter"> select <include refid="issueFilterColumns"/> from issue_filters filters <where> filters.shared=${_true} - and filters.user_login<>#{user} + and filters.user_login<>#{userLogin} + </where> + </select> + + <select id="selectSharedWithoutUserFiltersByName" parameterType="String" resultType="IssueFilter"> + select <include refid="issueFilterColumns"/> + from issue_filters filters + <where> + filters.shared=${_true} + and filters.user_login<>#{userLogin} + and filters.name=#{name} + <if test="existingId != null"> + and filters.id<>#{existingId} + </if> </where> </select> diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml index 06fa350595e..a6b5c76457f 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml @@ -217,7 +217,7 @@ <!-- Oracle --> <select id="selectIssues" parameterType="map" resultType="Issue" fetchSize="100000" databaseId="oracle"> - select * from (select i.id + select id from (select i.id <include refid="sortColumn"/> <include refid="selectQueryConditions"/> ) where rownum <= #{maxResults} diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueFilterTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueFilterSerializerTest.java index 4bf9d656f50..cc7f4a7dabd 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueFilterTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueFilterSerializerTest.java @@ -29,37 +29,12 @@ import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Maps.newLinkedHashMap; import static org.fest.assertions.Assertions.assertThat; -public class DefaultIssueFilterTest { +public class IssueFilterSerializerTest { - DefaultIssueFilter issueFilter = new DefaultIssueFilter(); + IssueFilterSerializer issueFilterSerializer = new IssueFilterSerializer(); @Test - public void should_convert_data_to_map() { - String data = "issues=ABCDE1234|severities=MAJOR,MINOR|resolved=true|pageSize=10|pageIndex=50"; - - Map<String, Object> map = issueFilter.dataAsMap(data); - - assertThat(map).hasSize(5); - assertThat(map.get("issues")).isEqualTo("ABCDE1234"); - assertThat(map.get("severities")).isInstanceOf(List.class); - assertThat((List<String>) map.get("severities")).contains("MAJOR", "MINOR"); - assertThat(map.get("resolved")).isEqualTo("true"); - assertThat(map.get("pageSize")).isEqualTo("10"); - assertThat(map.get("pageIndex")).isEqualTo("50"); - } - - @Test - public void should_remove_empty_value_when_converting_data_to_map() { - String data = "issues=ABCDE1234|severities="; - - Map<String, Object> map = issueFilter.dataAsMap(data); - - assertThat(map).hasSize(1); - assertThat(map.get("issues")).isEqualTo("ABCDE1234"); - } - - @Test - public void should_convert_map_to_data() { + public void should_serialize() { Map<String, Object> map = newLinkedHashMap(); map.put("issues", newArrayList("ABCDE1234")); map.put("severities", newArrayList("MAJOR", "MINOR")); @@ -67,21 +42,47 @@ public class DefaultIssueFilterTest { map.put("pageSize", 10l); map.put("pageIndex", 50); - String result = issueFilter.mapAsdata(map); + String result = issueFilterSerializer.serialize(map); assertThat(result).isEqualTo("issues=ABCDE1234|severities=MAJOR,MINOR|resolved=true|pageSize=10|pageIndex=50"); } @Test - public void should_remove_empty_value_when_converting_convert_map_to_data() { + public void should_remove_empty_value_when_serializing() { Map<String, Object> map = newLinkedHashMap(); map.put("issues", newArrayList("ABCDE1234")); map.put("resolved", null); map.put("pageSize", ""); - String result = issueFilter.mapAsdata(map); + String result = issueFilterSerializer.serialize(map); assertThat(result).isEqualTo("issues=ABCDE1234"); } + @Test + public void should_deserialize() { + String data = "issues=ABCDE1234|severities=MAJOR,MINOR|resolved=true|pageSize=10|pageIndex=50"; + + Map<String, Object> map = issueFilterSerializer.deserialize(data); + + assertThat(map).hasSize(5); + assertThat(map.get("issues")).isEqualTo("ABCDE1234"); + assertThat(map.get("severities")).isInstanceOf(List.class); + assertThat((List<String>) map.get("severities")).contains("MAJOR", "MINOR"); + assertThat(map.get("resolved")).isEqualTo("true"); + assertThat(map.get("pageSize")).isEqualTo("10"); + assertThat(map.get("pageIndex")).isEqualTo("50"); + } + + @Test + public void should_remove_empty_value_when_deserializing() { + String data = "issues=ABCDE1234|severities="; + + Map<String, Object> map = issueFilterSerializer.deserialize(data); + + assertThat(map).hasSize(1); + assertThat(map.get("issues")).isEqualTo("ABCDE1234"); + } + + } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java index dd832ecf550..5a2a714ec95 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java @@ -85,8 +85,23 @@ public class IssueFilterDaoTest extends AbstractDaoTestCase { public void should_select_shared() { setupData("shared"); - assertThat(dao.selectSharedForUser("michael")).hasSize(1); - assertThat(dao.selectSharedForUser("stephane")).isEmpty(); + assertThat(dao.selectSharedWithoutUserFilters("michael")).hasSize(1); + assertThat(dao.selectSharedWithoutUserFilters("stephane")).isEmpty(); + } + + @Test + public void should_select_shared_by_name() { + setupData("should_select_shared_by_name"); + + IssueFilterDto result = dao.selectSharedWithoutUserFiltersByName("Open issues", "stephane", null); + assertThat(result).isNotNull(); + assertThat(result.getId()).isEqualTo(3L); + assertThat(result.getUserLogin()).isEqualTo("michael"); + assertThat(result.isShared()).isTrue(); + assertThat(dao.selectSharedWithoutUserFiltersByName("Open issues", "stephane", 3L)).isNull(); + + assertThat(dao.selectSharedWithoutUserFiltersByName("Open issues", "michael", null)).isNull(); + assertThat(dao.selectSharedWithoutUserFiltersByName("Sonar issues", "stephane", null)).isNull(); } @Test diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_select_shared_by_name.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_select_shared_by_name.xml new file mode 100644 index 00000000000..0dc50ff3f04 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_select_shared_by_name.xml @@ -0,0 +1,27 @@ +<dataset> + + <issue_filters + id="1" + name="Sonar Issues" + user_login="stephane" + shared="[true]" + created_at="2013-06-10" + updated_at="2013-06-10" /> + + <issue_filters + id="2" + name="Open issues" + user_login="stephane" + shared="[false]" + created_at="2013-06-10" + updated_at="2013-06-10" /> + + <issue_filters + id="3" + name="Open issues" + user_login="michael" + shared="[true]" + created_at="2013-06-10" + updated_at="2013-06-10" /> + +</dataset> diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 50611a9dd39..92f199f1729 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -368,6 +368,10 @@ public class InternalRubyIssueService implements ServerComponent { return PublicRubyIssueService.toQuery(props); } + public IssueQuery toQuery(DefaultIssueFilter issueFilter) { + return toQuery(issueFilterService.deserializeIssueFilterQuery(issueFilter)); + } + public DefaultIssueFilter findIssueFilter(Long id) { return issueFilterService.findById(id, UserSession.get()); } @@ -376,8 +380,8 @@ public class InternalRubyIssueService implements ServerComponent { return issueFilterService.findByUser(UserSession.get()); } - public DefaultIssueFilter createFilterFromMap(Map<String, Object> mapData) { - return new DefaultIssueFilter(mapData); + public String serializeFilterQuery(Map<String, Object> filterQuery){ + return issueFilterService.serializeFilterQuery(filterQuery); } /** @@ -434,10 +438,10 @@ public class InternalRubyIssueService implements ServerComponent { /** * Update issue filter data */ - public Result<DefaultIssueFilter> updateIssueFilterData(Long issueFilterId, Map<String, Object> data) { + public Result<DefaultIssueFilter> updateIssueFilterQuery(Long issueFilterId, Map<String, Object> data) { Result<DefaultIssueFilter> result = Result.of(); try { - result.set(issueFilterService.updateData(issueFilterId, data, UserSession.get())); + result.set(issueFilterService.updateFilterQuery(issueFilterId, data, UserSession.get())); } catch (Exception e) { result.addError(e.getMessage()); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java index 594e8c9fa40..63b7ae1eb03 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java @@ -28,6 +28,7 @@ import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.IssueQueryResult; import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultIssueFilter; +import org.sonar.core.issue.IssueFilterSerializer; import org.sonar.core.issue.db.IssueFilterDao; import org.sonar.core.issue.db.IssueFilterDto; import org.sonar.core.issue.db.IssueFilterFavouriteDao; @@ -49,12 +50,15 @@ public class IssueFilterService implements ServerComponent { private final IssueFilterFavouriteDao issueFilterFavouriteDao; private final IssueFinder issueFinder; private final AuthorizationDao authorizationDao; + private final IssueFilterSerializer issueFilterSerializer; - public IssueFilterService(IssueFilterDao issueFilterDao, IssueFilterFavouriteDao issueFilterFavouriteDao, IssueFinder issueFinder, AuthorizationDao authorizationDao) { + public IssueFilterService(IssueFilterDao issueFilterDao, IssueFilterFavouriteDao issueFilterFavouriteDao, IssueFinder issueFinder, AuthorizationDao authorizationDao, + IssueFilterSerializer issueFilterSerializer) { this.issueFilterDao = issueFilterDao; this.issueFilterFavouriteDao = issueFilterFavouriteDao; this.issueFinder = issueFinder; this.authorizationDao = authorizationDao; + this.issueFilterSerializer = issueFilterSerializer; } public IssueQueryResult execute(IssueQuery issueQuery) { @@ -65,7 +69,7 @@ public class IssueFilterService implements ServerComponent { IssueFilterDto issueFilterDto = findIssueFilter(issueFilterId, userSession); DefaultIssueFilter issueFilter = issueFilterDto.toIssueFilter(); - IssueQuery issueQuery = PublicRubyIssueService.toQuery(issueFilter.dataAsMap()); + IssueQuery issueQuery = PublicRubyIssueService.toQuery(deserializeIssueFilterQuery(issueFilter)); return issueFinder.find(issueQuery); } @@ -85,25 +89,25 @@ public class IssueFilterService implements ServerComponent { public DefaultIssueFilter save(DefaultIssueFilter issueFilter, UserSession userSession) { verifyLoggedIn(userSession); issueFilter.setUser(userSession.login()); - verifyNameIsNotAlreadyUsed(issueFilter, userSession); + validateFilter(issueFilter, userSession); return insertIssueFilter(issueFilter, userSession.login()); } public DefaultIssueFilter update(DefaultIssueFilter issueFilter, UserSession userSession) { IssueFilterDto issueFilterDto = findIssueFilter(issueFilter.id(), userSession); verifyCurrentUserCanModifyFilter(issueFilterDto, userSession); - verifyNameIsNotAlreadyUsed(issueFilter, userSession); + validateFilter(issueFilter, userSession); issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); return issueFilter; } - public DefaultIssueFilter updateData(Long issueFilterId, Map<String, Object> mapData, UserSession userSession) { + public DefaultIssueFilter updateFilterQuery(Long issueFilterId, Map<String, Object> filterQuery, UserSession userSession) { IssueFilterDto issueFilterDto = findIssueFilter(issueFilterId, userSession); verifyCurrentUserCanModifyFilter(issueFilterDto, userSession); DefaultIssueFilter issueFilter = issueFilterDto.toIssueFilter(); - issueFilter.setData(mapData); + issueFilter.setData(serializeFilterQuery(filterQuery)); issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); return issueFilter; } @@ -120,14 +124,14 @@ public class IssueFilterService implements ServerComponent { IssueFilterDto issueFilterDtoToCopy = findIssueFilter(issueFilterIdToCopy, userSession); issueFilter.setUser(userSession.login()); issueFilter.setData(issueFilterDtoToCopy.getData()); - verifyNameIsNotAlreadyUsed(issueFilter, userSession); + validateFilter(issueFilter, userSession); return insertIssueFilter(issueFilter, userSession.login()); } public List<DefaultIssueFilter> findSharedFilters(UserSession userSession) { if (userSession.isLoggedIn() && userSession.login() != null) { - return toIssueFilters(issueFilterDao.selectSharedForUser(userSession.login())); + return toIssueFilters(issueFilterDao.selectSharedWithoutUserFilters(userSession.login())); } return Collections.emptyList(); } @@ -160,6 +164,14 @@ public class IssueFilterService implements ServerComponent { return issueFilterDto; } + public String serializeFilterQuery(Map<String, Object> filterQuery){ + return issueFilterSerializer.serialize(filterQuery); + } + + public Map<String, Object> deserializeIssueFilterQuery(DefaultIssueFilter issueFilter){ + return issueFilterSerializer.deserialize(issueFilter.data()); + } + private void verifyLoggedIn(UserSession userSession) { if (!userSession.isLoggedIn() && userSession.login() != null) { throw new IllegalStateException("User is not logged in"); @@ -180,10 +192,13 @@ public class IssueFilterService implements ServerComponent { } } - private void verifyNameIsNotAlreadyUsed(DefaultIssueFilter issueFilter, UserSession userSession) { + private void validateFilter(DefaultIssueFilter issueFilter, UserSession userSession) { if (issueFilterDao.selectByNameAndUser(issueFilter.name(), userSession.login(), issueFilter.id()) != null) { throw new IllegalArgumentException("Name already exists"); } + if (issueFilter.shared() && issueFilterDao.selectSharedWithoutUserFiltersByName(issueFilter.name(), userSession.login(), issueFilter.id()) != null) { + throw new IllegalArgumentException("Other users already share filters with the same name"); + } } private IssueFilterFavouriteDto findFavouriteIssueFilter(String user, Long issueFilterId) { diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index 1878f6de95c..37323956b6a 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -40,6 +40,7 @@ import org.sonar.core.config.Logback; import org.sonar.core.i18n.GwtI18n; import org.sonar.core.i18n.I18nManager; import org.sonar.core.i18n.RuleI18nManager; +import org.sonar.core.issue.IssueFilterSerializer; import org.sonar.core.issue.IssueNotifications; import org.sonar.core.issue.IssueUpdater; import org.sonar.core.issue.workflow.FunctionExecutor; @@ -273,6 +274,7 @@ public final class Platform { servicesContainer.addSingleton(IssueNotifications.class); servicesContainer.addSingleton(ActionService.class); servicesContainer.addSingleton(Actions.class); + servicesContainer.addSingleton(IssueFilterSerializer.class); servicesContainer.addSingleton(IssueFilterService.class); // rules diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb index 2d05a10d4d9..0ae9f832ddf 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb @@ -31,10 +31,9 @@ class IssuesController < ApplicationController def search if params[:id] @filter = find_filter(params[:id].to_i) - else - @filter = Internal.issues.createFilterFromMap(criteria_params) end - @criteria_params = Hash[@filter.dataAsMap] + + @first_search = criteria_params.empty? @issue_query = Internal.issues.toQuery(criteria_params) @issues_result = Internal.issues.execute(@issue_query) end @@ -45,8 +44,8 @@ class IssuesController < ApplicationController require_parameters :id @filter = find_filter(params[:id].to_i) - @criteria_params = Hash[@filter.dataAsMap] - @issue_query = Internal.issues.toQuery(@filter.dataAsMap) + @first_search = false + @issue_query = Internal.issues.toQuery(@filter) @issues_result = Internal.issues.execute(@issue_query) @unchanged = true @@ -63,7 +62,7 @@ class IssuesController < ApplicationController # GET /issues/save_as_form?[&criteria] def save_as_form - @filter = Internal.issues.createFilterFromMap(criteria_params) + @filter_query_serialized = Internal.issues.serializeFilterQuery(criteria_params) render :partial => 'issues/save_as_form' end @@ -87,7 +86,7 @@ class IssuesController < ApplicationController verify_post_request require_parameters :id - filter_result = Internal.issues.updateIssueFilterData(params[:id].to_i, criteria_params) + filter_result = Internal.issues.updateIssueFilterQuery(params[:id].to_i, criteria_params) if filter_result.ok @filter = filter_result.get() redirect_to :action => 'filter', :id => @filter.id.to_s @@ -95,6 +94,7 @@ class IssuesController < ApplicationController @errors = filter_result.errors @filter = find_filter(params[:id].to_i) + @issue_query = Internal.issues.toQuery(@filter.dataAsMap) @issues_result = Internal.issues.execute(@issue_query) @unchanged = true diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb index 4407f216bd7..fd2759e4c8b 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb @@ -1,5 +1,5 @@ <form id="save-as-filter-form" method="post" action="<%= ApplicationController.root_context -%>/issues/save_as"> - <input type="hidden" name="data" value="<%= params[:data] || u(@filter.data) -%>"> + <input type="hidden" name="data" value="<%= params[:data] || u(@filter_query_serialized) -%>"> <fieldset> <div class="modal-head"> <h2><%= message('issue_filter.save_filter') -%></h2> diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb index 695f50a8ff7..af3d073e3c0 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb @@ -2,11 +2,11 @@ <%= render :partial => 'display_errors' %> <div class="modal-field"> <label for="name"><%= message('issue_filter.form.name') -%> <em class="mandatory">*</em></label> - <input id="name" name="name" type="text" size="50" maxlength="100" value="<%= params[:name] || h(@filter.name) -%>" autofocus="autofocus"/> + <input id="name" name="name" type="text" size="50" maxlength="100" value="<%= params[:name] || (h(@filter.name) if @filter) -%>" autofocus="autofocus"/> </div> <div class="modal-field"> <label for="description"><%= message('issue_filter.form.description') -%></label> - <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= params[:description] || h(@filter.description) -%>"/> + <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= params[:description] || (h(@filter.description) if @filter) -%>"/> </div> <div class="modal-field"> <label for="shared"><%= message('issue_filter.form.share') -%></label> diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb index 4e7b9f1652c..4b49d74a11c 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb @@ -8,25 +8,25 @@ <%= render :partial => 'display_errors' %> <div class="line-block marginbottom10"> - <% if logged_in? && !@criteria_params.empty? %> + <% if logged_in? && !@first_search %> <ul class="operations"> - <% if @filter.id %> + <% if @filter && @filter.id %> <li><a id="copy" href="<%= url_for :action => 'copy_form', :id => @filter.id -%>" class="link-action open-modal"><%= message('copy') -%></a></li> <% end %> - <% if !defined?(@unchanged) && @filter.id && @filter.user == current_user.login %> + <% if !defined?(@unchanged) && @filter && @filter.id && @filter.user == current_user.login %> <li> <%= link_to message('save'), params.merge({:action => 'save', :id => @filter.id}), :class => 'link-action', :id => 'save', :method => :post -%> </li> <% end %> - <% unless @filter.id %> + <% unless @filter %> <li> - <a id="save-as" href="<%= url_for params.merge({:action => 'save_as_form', :id => @filter.id}) -%>" class="link-action open-modal"><%= message('save_as') -%></a> + <a id="save-as" href="<%= url_for params.merge({:action => 'save_as_form'}) -%>" class="link-action open-modal"><%= message('save_as') -%></a> </li> <% end %> </ul> <div class="page_title" id="filter-title"> - <% if @filter.id && @filter.name.present? %> + <% if @filter && @filter.id && @filter.name.present? %> <p> <span class="h3"><%= h @filter.name -%></span> <span class="note"> diff --git a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java index 30a501842f4..b5985bcb552 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java @@ -325,8 +325,8 @@ public class InternalRubyIssueServiceTest { @Test public void should_update_data() { Map<String, Object> data = newHashMap(); - service.updateIssueFilterData(10L, data); - verify(issueFilterService).updateData(eq(10L), eq(data), any(UserSession.class)); + service.updateIssueFilterQuery(10L, data); + verify(issueFilterService).updateFilterQuery(eq(10L), eq(data), any(UserSession.class)); } @Test diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java index 5a2aad576cd..30ea6d28682 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java @@ -27,6 +27,7 @@ import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultIssueFilter; +import org.sonar.core.issue.IssueFilterSerializer; import org.sonar.core.issue.db.IssueFilterDao; import org.sonar.core.issue.db.IssueFilterDto; import org.sonar.core.issue.db.IssueFilterFavouriteDao; @@ -52,6 +53,7 @@ public class IssueFilterServiceTest { private IssueFilterFavouriteDao issueFilterFavouriteDao; private IssueFinder issueFinder; private AuthorizationDao authorizationDao; + private IssueFilterSerializer issueFilterSerializer; private UserSession userSession; @@ -66,8 +68,9 @@ public class IssueFilterServiceTest { issueFilterFavouriteDao = mock(IssueFilterFavouriteDao.class); issueFinder = mock(IssueFinder.class); authorizationDao = mock(AuthorizationDao.class); + issueFilterSerializer = mock(IssueFilterSerializer.class); - service = new IssueFilterService(issueFilterDao, issueFilterFavouriteDao, issueFinder, authorizationDao); + service = new IssueFilterService(issueFilterDao, issueFilterFavouriteDao, issueFinder, authorizationDao, issueFilterSerializer); } @Test @@ -189,6 +192,20 @@ public class IssueFilterServiceTest { } @Test + public void should_not_save_shared_filter_if_name_already_used_by_shared_filter() { + when(issueFilterDao.selectByNameAndUser(eq("My Issue"), eq("john"), eq((Long) null))).thenReturn(null); + when(issueFilterDao.selectSharedWithoutUserFiltersByName(eq("My Issue"), eq("john"), eq((Long) null))).thenReturn(new IssueFilterDto()); + try { + DefaultIssueFilter issueFilter = new DefaultIssueFilter().setName("My Issue").setShared(true); + service.save(issueFilter, userSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Other users already share filters with the same name"); + } + verify(issueFilterDao, never()).insert(any(IssueFilterDto.class)); + } + + @Test public void should_update() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john")); @@ -252,12 +269,13 @@ public class IssueFilterServiceTest { @Test public void should_update_data() { + when(issueFilterSerializer.serialize(anyMap())).thenReturn("componentRoots=struts"); when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john")); Map<String, Object> data = newHashMap(); data.put("componentRoots", "struts"); - DefaultIssueFilter result = service.updateData(1L, data, userSession); + DefaultIssueFilter result = service.updateFilterQuery(1L, data, userSession); assertThat(result.data()).isEqualTo("componentRoots=struts"); verify(issueFilterDao).update(any(IssueFilterDto.class)); @@ -380,6 +398,9 @@ public class IssueFilterServiceTest { @Test public void should_execute_from_filter_id() { + Map<String, Object> map = newHashMap(); + map.put("componentRoots", "struts"); + when(issueFilterSerializer.deserialize(anyString())).thenReturn(map); when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john").setData("componentRoots=struts")); ArgumentCaptor<IssueQuery> issueQueryCaptor = ArgumentCaptor.forClass(IssueQuery.class); @@ -393,7 +414,7 @@ public class IssueFilterServiceTest { @Test public void should_find_shared_issue_filter() { - when(issueFilterDao.selectSharedForUser("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john").setShared(true))); + when(issueFilterDao.selectSharedWithoutUserFilters("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john").setShared(true))); List<DefaultIssueFilter> results = service.findSharedFilters(userSession); assertThat(results).hasSize(1); |