From 9707f7cb88c6042a5da87c27e3864f664b6567d7 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 1 Jul 2013 09:26:01 +0200 Subject: [PATCH] SONAR-4469 Favourite issue filters still visible even if unshared --- .../sonar/core/issue/db/IssueFilterDao.java | 28 +-- .../issue/db/IssueFilterFavouriteDao.java | 14 +- .../issue/db/IssueFilterFavouriteMapper.java | 9 +- .../core/issue/db/IssueFilterMapper.java | 13 +- .../issue/db/IssueFilterFavouriteMapper.xml | 7 +- .../sonar/core/issue/db/IssueFilterMapper.xml | 32 +--- .../core/issue/db/IssueFilterDaoTest.java | 31 +-- .../issue/db/IssueFilterFavouriteDaoTest.java | 39 ++-- .../should_select_shared_by_name.xml | 27 --- .../issue/InternalRubyIssueService.java | 10 +- .../server/issue/IssueFilterService.java | 177 +++++++++++------- .../issue/InternalRubyIssueServiceTest.java | 4 +- .../server/issue/IssueFilterServiceTest.java | 49 +++-- 13 files changed, 204 insertions(+), 236 deletions(-) delete mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_select_shared_by_name.xml 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 572d23e41ed..51ca69c6447 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 @@ -26,7 +26,6 @@ import org.sonar.api.ServerComponent; import org.sonar.core.persistence.MyBatis; import javax.annotation.CheckForNull; -import javax.annotation.Nullable; import java.util.List; @@ -52,16 +51,6 @@ public class IssueFilterDao implements BatchComponent, ServerComponent { } } - @CheckForNull - public IssueFilterDto selectByNameAndUser(String name, String user, @Nullable Long existingId) { - SqlSession session = mybatis.openSession(); - try { - return getMapper(session).selectByNameAndUser(name, user, existingId); - } finally { - MyBatis.closeQuietly(session); - } - } - public List selectByUser(String user) { SqlSession session = mybatis.openSession(); try { @@ -71,28 +60,19 @@ public class IssueFilterDao implements BatchComponent, ServerComponent { } } - public List selectByUserWithOnlyFavoriteFilters(String user) { - SqlSession session = mybatis.openSession(); - try { - return getMapper(session).selectByUserWithOnlyFavoriteFilters(user); - } finally { - MyBatis.closeQuietly(session); - } - } - - public List selectSharedWithoutUserFilters(String user) { + public List selectFavoriteFiltersByUser(String user) { SqlSession session = mybatis.openSession(); try { - return getMapper(session).selectSharedWithoutUserFilters(user); + return getMapper(session).selectFavoriteFiltersByUser(user); } finally { MyBatis.closeQuietly(session); } } - public IssueFilterDto selectSharedWithoutUserFiltersByName(String name, String user, @Nullable Long existingId) { + public List selectSharedFilters() { SqlSession session = mybatis.openSession(); try { - return getMapper(session).selectSharedWithoutUserFiltersByName(name, user, existingId); + return getMapper(session).selectSharedFilters(); } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java index 17e2b17b1de..1ee990aa254 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java @@ -25,6 +25,8 @@ import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; import org.sonar.core.persistence.MyBatis; +import java.util.List; + /** * @since 3.7 */ @@ -45,10 +47,10 @@ public class IssueFilterFavouriteDao implements BatchComponent, ServerComponent } } - public IssueFilterFavouriteDto selectByUserAndIssueFilterId(String user, Long issueFilterId) { + public List selectByFilterId(Long filterId) { SqlSession session = mybatis.openSession(); try { - return getMapper(session).selectByIssueFilterId(user, issueFilterId); + return getMapper(session).selectByFilterId(filterId); } finally { MyBatis.closeQuietly(session); } @@ -64,20 +66,20 @@ public class IssueFilterFavouriteDao implements BatchComponent, ServerComponent } } - public void delete(Long issueFilterFavouriteId) { + public void delete(Long id) { SqlSession session = mybatis.openSession(); try { - getMapper(session).delete(issueFilterFavouriteId); + getMapper(session).delete(id); session.commit(); } finally { MyBatis.closeQuietly(session); } } - public void deleteByIssueFilterId(Long issueFilterId) { + public void deleteByFilterId(Long filterId) { SqlSession session = mybatis.openSession(); try { - getMapper(session).deleteByIssueFilterId(issueFilterId); + getMapper(session).deleteByFilterId(filterId); session.commit(); } finally { MyBatis.closeQuietly(session); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java index d11fc9d1ca1..8890d613152 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java @@ -23,6 +23,8 @@ import org.apache.ibatis.annotations.Param; import javax.annotation.CheckForNull; +import java.util.List; + /** * @since 3.7 */ @@ -31,12 +33,11 @@ public interface IssueFilterFavouriteMapper { @CheckForNull IssueFilterFavouriteDto selectById(Long id); - @CheckForNull - IssueFilterFavouriteDto selectByIssueFilterId(@Param("userLogin") String userLogin, @Param("issueFilterId") Long issueFilterId); + List selectByFilterId(@Param("filterId") Long filterId); void insert(IssueFilterFavouriteDto filterFavourite); - void delete(Long issueFilterFavouriteId); + void delete(Long id); - void deleteByIssueFilterId(Long issueFilterId); + void deleteByFilterId(Long filterId); } 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 eb634af22b6..052ad7d743c 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 @@ -19,10 +19,7 @@ */ package org.sonar.core.issue.db; -import org.apache.ibatis.annotations.Param; - import javax.annotation.CheckForNull; -import javax.annotation.Nullable; import java.util.List; @@ -34,17 +31,11 @@ public interface IssueFilterMapper { @CheckForNull IssueFilterDto selectById(Long id); - @CheckForNull - IssueFilterDto selectByNameAndUser(@Param("name") String name, @Param("userLogin") String userLogin, @Nullable @Param("existingId") Long existingId); - List selectByUser(String user); - List selectByUserWithOnlyFavoriteFilters(String user); + List selectFavoriteFiltersByUser(String user); - List selectSharedWithoutUserFilters(String user); - - @CheckForNull - IssueFilterDto selectSharedWithoutUserFiltersByName(@Param("name") String name, @Param("userLogin") String userLogin, @Nullable @Param("existingId") Long existingId); + List selectSharedFilters(); void insert(IssueFilterDto filter); diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml index f8aaa53f945..15d9b8d9263 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml @@ -19,12 +19,11 @@ - select from issue_filter_favourites filter_favourites - filter_favourites.issue_filter_id=#{issueFilterId} - and filter_favourites.user_login=#{userLogin} + filter_favourites.issue_filter_id=#{filterId} @@ -37,7 +36,7 @@ delete from issue_filter_favourites where id=#{id} - + delete from issue_filter_favourites where issue_filter_id=#{issueFilterId} 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 21a2c87cfee..954c7af61f0 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 @@ -23,18 +23,6 @@ - - - select from issue_filters filters inner join issue_filter_favourites fav on fav.issue_filter_id = filters.id @@ -52,25 +40,11 @@ - - - select from issue_filters filters - filters.shared=${_true} - and filters.user_login<>#{userLogin} - and filters.name=#{name} - - and filters.id<>#{existingId} - + and filters.shared=${_true} 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 fd3eb8e7e93..1061e5764a5 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 @@ -50,17 +50,6 @@ public class IssueFilterDaoTest extends AbstractDaoTestCase { assertThat(dao.selectById(123L)).isNull(); } - @Test - public void should_select_by_name_and_user() { - setupData("shared"); - - IssueFilterDto filter = dao.selectByNameAndUser("Sonar Issues", "stephane", null); - assertThat(filter.getId()).isEqualTo(1L); - - filter = dao.selectByNameAndUser("Sonar Issues", "stephane", 1L); - assertThat(filter).isNull(); - } - @Test public void should_select_by_user() { setupData("should_select_by_user"); @@ -74,7 +63,7 @@ public class IssueFilterDaoTest extends AbstractDaoTestCase { public void should_select_by_user_with_only_favorite_filters() { setupData("should_select_by_user_with_only_favorite_filters"); - List results = dao.selectByUserWithOnlyFavoriteFilters("michael"); + List results = dao.selectFavoriteFiltersByUser("michael"); assertThat(results).hasSize(1); IssueFilterDto issueFilterDto = results.get(0); @@ -85,23 +74,7 @@ public class IssueFilterDaoTest extends AbstractDaoTestCase { public void should_select_shared() { setupData("shared"); - 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(); + assertThat(dao.selectSharedFilters()).hasSize(1); } @Test diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java index c41e1ab52a9..e1112415a42 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java @@ -24,6 +24,8 @@ import org.junit.Before; import org.junit.Test; import org.sonar.core.persistence.AbstractDaoTestCase; +import java.util.List; + import static org.fest.assertions.Assertions.assertThat; public class IssueFilterFavouriteDaoTest extends AbstractDaoTestCase { @@ -39,37 +41,40 @@ public class IssueFilterFavouriteDaoTest extends AbstractDaoTestCase { public void should_select_by_id() { setupData("shared"); - IssueFilterFavouriteDto issueFilterFavouriteDto = dao.selectById(1L); - assertThat(issueFilterFavouriteDto.getId()).isEqualTo(1L); - assertThat(issueFilterFavouriteDto.getUserLogin()).isEqualTo("stephane"); - assertThat(issueFilterFavouriteDto.getIssueFilterId()).isEqualTo(10L); - assertThat(issueFilterFavouriteDto.getCreatedAt()).isNotNull(); + IssueFilterFavouriteDto dto = dao.selectById(1L); + assertThat(dto.getId()).isEqualTo(1L); + assertThat(dto.getUserLogin()).isEqualTo("stephane"); + assertThat(dto.getIssueFilterId()).isEqualTo(10L); + assertThat(dto.getCreatedAt()).isNotNull(); assertThat(dao.selectById(999L)).isNull(); } @Test - public void should_select_by_issue_filter_id() { + public void should_select_by_filter_id() { setupData("shared"); - IssueFilterFavouriteDto issueFilterFavouriteDto = dao.selectByUserAndIssueFilterId("stephane", 10L); - assertThat(issueFilterFavouriteDto.getId()).isEqualTo(1L); - assertThat(issueFilterFavouriteDto.getUserLogin()).isEqualTo("stephane"); - assertThat(issueFilterFavouriteDto.getIssueFilterId()).isEqualTo(10L); - assertThat(issueFilterFavouriteDto.getCreatedAt()).isNotNull(); + List dtos = dao.selectByFilterId(11L); + assertThat(dtos).hasSize(1); + IssueFilterFavouriteDto dto = dtos.get(0); + assertThat(dto.getId()).isEqualTo(2L); + assertThat(dto.getUserLogin()).isEqualTo("stephane"); + assertThat(dto.getIssueFilterId()).isEqualTo(11L); + assertThat(dto.getCreatedAt()).isNotNull(); - assertThat(dao.selectByUserAndIssueFilterId("stephane", 999L)).isNull(); + assertThat(dao.selectByFilterId(10L)).hasSize(2); + assertThat(dao.selectByFilterId(999L)).isEmpty(); } @Test public void should_insert() { setupData("shared"); - IssueFilterFavouriteDto issueFilterFavouriteDto = new IssueFilterFavouriteDto(); - issueFilterFavouriteDto.setUserLogin("arthur"); - issueFilterFavouriteDto.setIssueFilterId(11L); + IssueFilterFavouriteDto dto = new IssueFilterFavouriteDto(); + dto.setUserLogin("arthur"); + dto.setIssueFilterId(11L); - dao.insert(issueFilterFavouriteDto); + dao.insert(dto); checkTables("should_insert", new String[]{"created_at"}, "issue_filter_favourites"); } @@ -87,7 +92,7 @@ public class IssueFilterFavouriteDaoTest extends AbstractDaoTestCase { public void should_delete_by_issue_filter_id() { setupData("shared"); - dao.deleteByIssueFilterId(10l); + dao.deleteByFilterId(10l); checkTables("should_delete_by_issue_filter_id", new String[]{"created_at"}, "issue_filter_favourites"); } 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 deleted file mode 100644 index 0dc50ff3f04..00000000000 --- a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_select_shared_by_name.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - 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 22b0c9e8676..f52804e5da0 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 @@ -398,7 +398,7 @@ public class InternalRubyIssueService implements ServerComponent { public boolean isUserAuthorized(DefaultIssueFilter issueFilter) { try { UserSession userSession = UserSession.get(); - String user = issueFilterService.getNotNullLogin(userSession); + String user = issueFilterService.getLoggedLogin(userSession); issueFilterService.verifyCurrentUserCanReadFilter(issueFilter, user); return true; } catch (Exception e) { @@ -542,7 +542,7 @@ public class InternalRubyIssueService implements ServerComponent { } @VisibleForTesting - Result createIssueFilterResult(Map params, boolean showCheckId, boolean showCheckUser) { + Result createIssueFilterResult(Map params, boolean checkId, boolean checkUser) { Result result = Result.of(); String id = params.get("id"); @@ -553,10 +553,10 @@ public class InternalRubyIssueService implements ServerComponent { Boolean sharedParam = RubyUtils.toBoolean(params.get("shared")); boolean shared = sharedParam != null ? sharedParam : false; - if (showCheckId) { + if (checkId) { checkMandatoryParameter(id, "id", result); } - if (showCheckUser) { + if (checkUser) { checkMandatoryParameter(user, "user", result); } checkMandatorySizeParameter(name, "name", 100, result); @@ -578,7 +578,7 @@ public class InternalRubyIssueService implements ServerComponent { } public List findSharedFiltersForCurrentUser() { - return issueFilterService.findSharedFilters(UserSession.get()); + return issueFilterService.findSharedFiltersWithoutUserFilters(UserSession.get()); } public List findFavouriteIssueFiltersForCurrentUser() { 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 afdd7f6841e..77568fbd46a 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 @@ -50,32 +50,32 @@ import static com.google.common.collect.Lists.newArrayList; */ public class IssueFilterService implements ServerComponent { - private final IssueFilterDao issueFilterDao; - private final IssueFilterFavouriteDao issueFilterFavouriteDao; - private final IssueFinder issueFinder; + private final IssueFilterDao filterDao; + private final IssueFilterFavouriteDao favouriteDao; + private final IssueFinder finder; private final AuthorizationDao authorizationDao; - private final IssueFilterSerializer issueFilterSerializer; + private final IssueFilterSerializer serializer; - public IssueFilterService(IssueFilterDao issueFilterDao, IssueFilterFavouriteDao issueFilterFavouriteDao, IssueFinder issueFinder, AuthorizationDao authorizationDao, - IssueFilterSerializer issueFilterSerializer) { - this.issueFilterDao = issueFilterDao; - this.issueFilterFavouriteDao = issueFilterFavouriteDao; - this.issueFinder = issueFinder; + public IssueFilterService(IssueFilterDao filterDao, IssueFilterFavouriteDao favouriteDao, IssueFinder finder, AuthorizationDao authorizationDao, + IssueFilterSerializer serializer) { + this.filterDao = filterDao; + this.favouriteDao = favouriteDao; + this.finder = finder; this.authorizationDao = authorizationDao; - this.issueFilterSerializer = issueFilterSerializer; + this.serializer = serializer; } public IssueFilterResult execute(IssueQuery issueQuery) { - return createIssueFilterResult(issueFinder.find(issueQuery), issueQuery); + return createIssueFilterResult(finder.find(issueQuery), issueQuery); } public DefaultIssueFilter find(Long id, UserSession userSession) { - return findIssueFilterDto(id, getNotNullLogin(userSession)).toIssueFilter(); + return findIssueFilterDto(id, getLoggedLogin(userSession)).toIssueFilter(); } @CheckForNull public DefaultIssueFilter findById(Long id) { - IssueFilterDto issueFilterDto = issueFilterDao.selectById(id); + IssueFilterDto issueFilterDto = filterDao.selectById(id); if (issueFilterDto != null) { return issueFilterDto.toIssueFilter(); } @@ -83,73 +83,88 @@ public class IssueFilterService implements ServerComponent { } public List findByUser(UserSession userSession) { - return toIssueFilters(issueFilterDao.selectByUser(getNotNullLogin(userSession))); + return toIssueFilters(selectUserIssueFilters(getLoggedLogin(userSession))); } public DefaultIssueFilter save(DefaultIssueFilter issueFilter, UserSession userSession) { - String user = getNotNullLogin(userSession); + String user = getLoggedLogin(userSession); issueFilter.setUser(user); - validateFilter(issueFilter, user); + validateFilter(issueFilter); return insertIssueFilter(issueFilter, user); } public DefaultIssueFilter update(DefaultIssueFilter issueFilter, UserSession userSession) { - String user = getNotNullLogin(userSession); - IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilter.id(), user); - verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), user); - if(!issueFilterDto.getUserLogin().equals(issueFilter.user())) { - verifyCurrentUserCanChangeFilterOwnership(user); + String login = getLoggedLogin(userSession); + IssueFilterDto existingFilterDto = findIssueFilterDto(issueFilter.id(), login); + verifyCurrentUserCanModifyFilter(existingFilterDto.toIssueFilter(), login); + if(!existingFilterDto.getUserLogin().equals(issueFilter.user())) { + verifyCurrentUserCanChangeFilterOwnership(login); } - validateFilter(issueFilter, user); - - issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); + validateFilter(issueFilter); + deleteOtherFavoriteFiltersIfFilterBecomeUnshared(existingFilterDto, issueFilter); + filterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); return issueFilter; } + private void deleteOtherFavoriteFiltersIfFilterBecomeUnshared(IssueFilterDto existingFilterDto, DefaultIssueFilter issueFilter){ + if (existingFilterDto.isShared() && !issueFilter.shared()) { + for (IssueFilterFavouriteDto favouriteDto : selectFavouriteFilters(existingFilterDto.getId())) { + if (!favouriteDto.getUserLogin().equals(issueFilter.user())) { + deleteFavouriteIssueFilter(favouriteDto); + } + } + } + } + public DefaultIssueFilter updateFilterQuery(Long issueFilterId, Map filterQuery, UserSession userSession) { - String user = getNotNullLogin(userSession); - IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilterId, user); - verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), user); + String login = getLoggedLogin(userSession); + IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilterId, login); + verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), login); DefaultIssueFilter issueFilter = issueFilterDto.toIssueFilter(); issueFilter.setData(serializeFilterQuery(filterQuery)); - issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); + filterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); return issueFilter; } public void delete(Long issueFilterId, UserSession userSession) { - String user = getNotNullLogin(userSession); - IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilterId, user); - verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), user); + String login = getLoggedLogin(userSession); + IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilterId, login); + verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), login); deleteFavouriteIssueFilters(issueFilterDto); - issueFilterDao.delete(issueFilterId); + filterDao.delete(issueFilterId); } public DefaultIssueFilter copy(Long issueFilterIdToCopy, DefaultIssueFilter issueFilter, UserSession userSession) { - String user = getNotNullLogin(userSession); - IssueFilterDto issueFilterDtoToCopy = findIssueFilterDto(issueFilterIdToCopy, user); - issueFilter.setUser(user); + String login = getLoggedLogin(userSession); + IssueFilterDto issueFilterDtoToCopy = findIssueFilterDto(issueFilterIdToCopy, login); + issueFilter.setUser(login); issueFilter.setData(issueFilterDtoToCopy.getData()); - validateFilter(issueFilter, user); - - return insertIssueFilter(issueFilter, user); + validateFilter(issueFilter); + return insertIssueFilter(issueFilter, login); } - public List findSharedFilters(UserSession userSession) { - return toIssueFilters(issueFilterDao.selectSharedWithoutUserFilters(getNotNullLogin(userSession))); + public List findSharedFiltersWithoutUserFilters(UserSession userSession) { + final String login = getLoggedLogin(userSession); + return toIssueFilters(newArrayList(Iterables.filter(selectSharedFilters(), new Predicate() { + @Override + public boolean apply(IssueFilterDto input) { + return !input.getUserLogin().equals(login); + } + }))); } public List findFavoriteFilters(UserSession userSession) { - return toIssueFilters(issueFilterDao.selectByUserWithOnlyFavoriteFilters(getNotNullLogin(userSession))); + return toIssueFilters(filterDao.selectFavoriteFiltersByUser(getLoggedLogin(userSession))); } - public void toggleFavouriteIssueFilter(Long issueFilterId, UserSession userSession) { - String user = getNotNullLogin(userSession); - findIssueFilterDto(issueFilterId, user); - IssueFilterFavouriteDto issueFilterFavouriteDto = findFavouriteIssueFilter(user, issueFilterId); + public void toggleFavouriteIssueFilter(Long filterId, UserSession userSession) { + String user = getLoggedLogin(userSession); + findIssueFilterDto(filterId, user); + IssueFilterFavouriteDto issueFilterFavouriteDto = selectFavouriteFilterForUser(filterId, user); if (issueFilterFavouriteDto == null) { - addFavouriteIssueFilter(issueFilterId, user); + addFavouriteIssueFilter(filterId, user); } else { deleteFavouriteIssueFilter(issueFilterFavouriteDto); } @@ -162,24 +177,24 @@ public class IssueFilterService implements ServerComponent { return IssueFilterParameters.ALL_WITHOUT_PAGINATION.contains(input.getKey()); } }); - return issueFilterSerializer.serialize(filterQueryFiltered); + return serializer.serialize(filterQueryFiltered); } public Map deserializeIssueFilterQuery(DefaultIssueFilter issueFilter) { - return issueFilterSerializer.deserialize(issueFilter.data()); + return serializer.deserialize(issueFilter.data()); } - private IssueFilterDto findIssueFilterDto(Long id, String user) { - IssueFilterDto issueFilterDto = issueFilterDao.selectById(id); + private IssueFilterDto findIssueFilterDto(Long id, String login) { + IssueFilterDto issueFilterDto = filterDao.selectById(id); if (issueFilterDto == null) { // TODO throw 404 throw new IllegalArgumentException("Filter not found: " + id); } - verifyCurrentUserCanReadFilter(issueFilterDto.toIssueFilter(), user); + verifyCurrentUserCanReadFilter(issueFilterDto.toIssueFilter(), login); return issueFilterDto; } - String getNotNullLogin(UserSession userSession) { + String getLoggedLogin(UserSession userSession) { String user = userSession.login(); if (!userSession.isLoggedIn() && user != null) { throw new IllegalStateException("User is not logged in"); @@ -187,8 +202,8 @@ public class IssueFilterService implements ServerComponent { return user; } - void verifyCurrentUserCanReadFilter(DefaultIssueFilter issueFilter, String user) { - if (!issueFilter.user().equals(user) && !issueFilter.shared()) { + void verifyCurrentUserCanReadFilter(DefaultIssueFilter issueFilter, String login) { + if (!issueFilter.user().equals(login) && !issueFilter.shared()) { // TODO throw unauthorized throw new IllegalStateException("User is not authorized to read this filter"); } @@ -208,37 +223,71 @@ public class IssueFilterService implements ServerComponent { } } - private void validateFilter(DefaultIssueFilter issueFilter, String user) { - if (issueFilterDao.selectByNameAndUser(issueFilter.name(), user, issueFilter.id()) != null) { + private void validateFilter(final DefaultIssueFilter issueFilter) { + List userFilters = selectUserIssueFilters(issueFilter.user()); + IssueFilterDto userFilterSameName = findFilterWithSameName(userFilters, issueFilter.name()); + if (userFilterSameName != null && userFilterSameName.getId() != issueFilter.id()) { throw new IllegalArgumentException("Name already exists"); } - if (issueFilter.shared() && issueFilterDao.selectSharedWithoutUserFiltersByName(issueFilter.name(), user, issueFilter.id()) != null) { - throw new IllegalArgumentException("Other users already share filters with the same name"); + if (issueFilter.shared()) { + List sharedFilters = selectSharedFilters(); + IssueFilterDto sharedFilterWithSameName = findFilterWithSameName(sharedFilters, issueFilter.name()); + if (sharedFilterWithSameName != null && !sharedFilterWithSameName.getUserLogin().equals(issueFilter.user())) { + throw new IllegalArgumentException("Other users already share filters with the same name"); + } } } - private IssueFilterFavouriteDto findFavouriteIssueFilter(String user, Long issueFilterId) { - return issueFilterFavouriteDao.selectByUserAndIssueFilterId(user, issueFilterId); + @CheckForNull + private IssueFilterFavouriteDto selectFavouriteFilterForUser(Long filterId, final String user) { + return Iterables.find(selectFavouriteFilters(filterId), new Predicate() { + @Override + public boolean apply(IssueFilterFavouriteDto input) { + return input.getUserLogin().equals(user); + } + }, null); + } + + private List selectFavouriteFilters(Long filterId) { + return favouriteDao.selectByFilterId(filterId); + } + + private List selectUserIssueFilters(String user){ + return filterDao.selectByUser(user); + } + + private List selectSharedFilters(){ + return filterDao.selectSharedFilters(); + } + + @CheckForNull + private IssueFilterDto findFilterWithSameName(List dtos, final String name){ + return Iterables.find(dtos, new Predicate() { + @Override + public boolean apply(IssueFilterDto input) { + return input.getName().equals(name); + } + }, null); } private void addFavouriteIssueFilter(Long issueFilterId, String user) { IssueFilterFavouriteDto issueFilterFavouriteDto = new IssueFilterFavouriteDto() .setIssueFilterId(issueFilterId) .setUserLogin(user); - issueFilterFavouriteDao.insert(issueFilterFavouriteDto); + favouriteDao.insert(issueFilterFavouriteDto); } private void deleteFavouriteIssueFilter(IssueFilterFavouriteDto issueFilterFavouriteDto) { - issueFilterFavouriteDao.delete(issueFilterFavouriteDto.getId()); + favouriteDao.delete(issueFilterFavouriteDto.getId()); } private void deleteFavouriteIssueFilters(IssueFilterDto issueFilterDto) { - issueFilterFavouriteDao.deleteByIssueFilterId(issueFilterDto.getId()); + favouriteDao.deleteByFilterId(issueFilterDto.getId()); } private DefaultIssueFilter insertIssueFilter(DefaultIssueFilter issueFilter, String user) { IssueFilterDto issueFilterDto = IssueFilterDto.toIssueFilter(issueFilter); - issueFilterDao.insert(issueFilterDto); + filterDao.insert(issueFilterDto); addFavouriteIssueFilter(issueFilterDto.getId(), user); return issueFilterDto.toIssueFilter(); } 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 296a64778c7..b9fe11d8b86 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 @@ -564,7 +564,7 @@ public class InternalRubyIssueServiceTest { @Test public void should_find_shared_issue_filters() { service.findSharedFiltersForCurrentUser(); - verify(issueFilterService).findSharedFilters(any(UserSession.class)); + verify(issueFilterService).findSharedFiltersWithoutUserFilters(any(UserSession.class)); } @Test @@ -592,7 +592,7 @@ public class InternalRubyIssueServiceTest { public void should_check_is_user_is_authorized_to_see_issue_filter() { DefaultIssueFilter issueFilter = new DefaultIssueFilter(); service.isUserAuthorized(issueFilter); - verify(issueFilterService).getNotNullLogin(any(UserSession.class)); + verify(issueFilterService).getLoggedLogin(any(UserSession.class)); verify(issueFilterService).verifyCurrentUserCanReadFilter(eq(issueFilter), anyString()); } 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 c3ed479cc3f..75cb12460ea 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 @@ -39,6 +39,7 @@ import org.sonar.core.user.AuthorizationDao; import org.sonar.server.user.MockUserSession; import org.sonar.server.user.UserSession; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -197,7 +198,7 @@ public class IssueFilterServiceTest { @Test public void should_not_save_if_name_already_used() { - when(issueFilterDao.selectByNameAndUser(eq("My Issue"), eq("john"), eq((Long) null))).thenReturn(new IssueFilterDto()); + when(issueFilterDao.selectByUser(eq("john"))).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue"))); try { DefaultIssueFilter issueFilter = new DefaultIssueFilter().setName("My Issue"); service.save(issueFilter, userSession); @@ -210,10 +211,10 @@ 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()); + when(issueFilterDao.selectByUser(eq("john"))).thenReturn(Collections.emptyList()); + when(issueFilterDao.selectSharedFilters()).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("henry").setShared(true))); + DefaultIssueFilter issueFilter = new DefaultIssueFilter().setName("My Issue").setShared(true); try { - DefaultIssueFilter issueFilter = new DefaultIssueFilter().setName("My Issue").setShared(true); service.save(issueFilter, userSession); fail(); } catch (Exception e) { @@ -233,7 +234,22 @@ public class IssueFilterServiceTest { } @Test - public void should_update_shared_filter_if_admin() { + public void should_remove_other_favorite_filters_if_filter_become_unshared() { + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john").setShared(true)); + IssueFilterFavouriteDto userFavouriteDto = new IssueFilterFavouriteDto().setId(10L).setUserLogin("john").setIssueFilterId(1L); + IssueFilterFavouriteDto otherFavouriteDto = new IssueFilterFavouriteDto().setId(11L).setUserLogin("arthur").setIssueFilterId(1L); + when(issueFilterFavouriteDao.selectByFilterId(1L)).thenReturn(newArrayList(userFavouriteDto, otherFavouriteDto)); + + DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter").setUser("john").setShared(false), userSession); + assertThat(result.name()).isEqualTo("My New Filter"); + + verify(issueFilterDao).update(any(IssueFilterDto.class)); + verify(issueFilterFavouriteDao).delete(11L); + verify(issueFilterFavouriteDao, never()).delete(10L); + } + + @Test + public void should_update_other_shared_filter_if_admin() { when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(UserRole.ADMIN)); when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("arthur").setShared(true)); @@ -274,7 +290,7 @@ public class IssueFilterServiceTest { @Test public void should_not_update_if_name_already_used() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john")); - when(issueFilterDao.selectByNameAndUser("My Issue", "john", 1L)).thenReturn(new IssueFilterDto()); + when(issueFilterDao.selectByUser(eq("john"))).thenReturn(newArrayList(new IssueFilterDto().setId(2L).setName("My Issue"))); try { service.update(new DefaultIssueFilter().setId(1L).setUser("john").setName("My Issue"), userSession); @@ -345,12 +361,12 @@ public class IssueFilterServiceTest { @Test public void should_delete_favorite_filter_on_delete() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("john")); - when(issueFilterFavouriteDao.selectByUserAndIssueFilterId("john", 1L)).thenReturn(new IssueFilterFavouriteDto().setId(10L).setUserLogin("john").setIssueFilterId(1L)); + when(issueFilterFavouriteDao.selectByFilterId(1L)).thenReturn(newArrayList(new IssueFilterFavouriteDto().setId(10L).setUserLogin("john").setIssueFilterId(1L))); service.delete(1L, userSession); verify(issueFilterDao).delete(1L); - verify(issueFilterFavouriteDao).deleteByIssueFilterId(1L); + verify(issueFilterFavouriteDao).deleteByFilterId(1L); } @Test @@ -450,15 +466,20 @@ public class IssueFilterServiceTest { @Test public void should_find_shared_issue_filter() { - when(issueFilterDao.selectSharedWithoutUserFilters("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john").setShared(true))); + when(issueFilterDao.selectSharedFilters()).thenReturn(newArrayList( + new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john").setShared(true), + new IssueFilterDto().setId(2L).setName("Project Issues").setUserLogin("arthur").setShared(true) + )); - List results = service.findSharedFilters(userSession); + List results = service.findSharedFiltersWithoutUserFilters(userSession); assertThat(results).hasSize(1); + DefaultIssueFilter filter = results.get(0); + assertThat(filter.name()).isEqualTo("Project Issues"); } @Test public void should_find_favourite_issue_filter() { - when(issueFilterDao.selectByUserWithOnlyFavoriteFilters("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john"))); + when(issueFilterDao.selectFavoriteFiltersByUser("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john"))); List results = service.findFavoriteFilters(userSession); assertThat(results).hasSize(1); @@ -480,7 +501,7 @@ public class IssueFilterServiceTest { public void should_add_favourite_issue_filter_id() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("john").setData("componentRoots=struts")); // The filter is not in the favorite list --> add to favorite - when(issueFilterFavouriteDao.selectByUserAndIssueFilterId("john", 1L)).thenReturn(null); + when(issueFilterFavouriteDao.selectByFilterId(1L)).thenReturn(Collections.emptyList()); ArgumentCaptor issueFilterFavouriteDtoCaptor = ArgumentCaptor.forClass(IssueFilterFavouriteDto.class); service.toggleFavouriteIssueFilter(1L, userSession); @@ -495,7 +516,7 @@ public class IssueFilterServiceTest { public void should_add_favourite_on_shared_filter() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(true)); // The filter is not in the favorite list --> add to favorite - when(issueFilterFavouriteDao.selectByUserAndIssueFilterId("john", 1L)).thenReturn(null); + when(issueFilterFavouriteDao.selectByFilterId(1L)).thenReturn(Collections.emptyList()); ArgumentCaptor issueFilterFavouriteDtoCaptor = ArgumentCaptor.forClass(IssueFilterFavouriteDto.class); service.toggleFavouriteIssueFilter(1L, userSession); @@ -510,7 +531,7 @@ public class IssueFilterServiceTest { public void should_delete_favourite_issue_filter_id() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("john").setData("componentRoots=struts")); // The filter is in the favorite list --> remove favorite - when(issueFilterFavouriteDao.selectByUserAndIssueFilterId("john", 1L)).thenReturn(new IssueFilterFavouriteDto().setId(10L).setUserLogin("john").setIssueFilterId(1L)); + when(issueFilterFavouriteDao.selectByFilterId(1L)).thenReturn(newArrayList(new IssueFilterFavouriteDto().setId(10L).setUserLogin("john").setIssueFilterId(1L))); service.toggleFavouriteIssueFilter(1L, userSession); verify(issueFilterFavouriteDao).delete(10L); -- 2.39.5