]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4469 Favourite issue filters still visible even if unshared
authorJulien Lancelot <julien.lancelot@gmail.com>
Mon, 1 Jul 2013 07:26:01 +0000 (09:26 +0200)
committerJulien Lancelot <julien.lancelot@gmail.com>
Tue, 2 Jul 2013 14:47:54 +0000 (16:47 +0200)
13 files changed:
sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java
sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java
sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java
sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java
sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml
sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml
sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java
sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_select_shared_by_name.xml [deleted file]
sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java
sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java

index 572d23e41edaaf9f223ec89eed21c6346e972a50..51ca69c644720ea3f2896d3a366b792b420c6af4 100644 (file)
@@ -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<IssueFilterDto> selectByUser(String user) {
     SqlSession session = mybatis.openSession();
     try {
@@ -71,28 +60,19 @@ public class IssueFilterDao implements BatchComponent, ServerComponent {
     }
   }
 
-  public List<IssueFilterDto> selectByUserWithOnlyFavoriteFilters(String user) {
-    SqlSession session = mybatis.openSession();
-    try {
-      return getMapper(session).selectByUserWithOnlyFavoriteFilters(user);
-    } finally {
-      MyBatis.closeQuietly(session);
-    }
-  }
-
-  public List<IssueFilterDto> selectSharedWithoutUserFilters(String user) {
+  public List<IssueFilterDto> 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<IssueFilterDto> selectSharedFilters() {
     SqlSession session = mybatis.openSession();
     try {
-      return getMapper(session).selectSharedWithoutUserFiltersByName(name, user, existingId);
+      return getMapper(session).selectSharedFilters();
     } finally {
       MyBatis.closeQuietly(session);
     }
index 17e2b17b1dede8af4d44dae3bf4df4d79e7f5bae..1ee990aa254f9f58bc45e782f04b9cf43206a339 100644 (file)
@@ -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<IssueFilterFavouriteDto> 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);
index d11fc9d1ca1bd10e3a27cd69a7f3b8c0cf928ca3..8890d6131529eeeb2a453e195ce390cb38788dbc 100644 (file)
@@ -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<IssueFilterFavouriteDto> 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);
 }
index eb634af22b6107331df946a14de0dbb049c12ee4..052ad7d743ccfd228f89296bf645aafdcc86d120 100644 (file)
  */
 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<IssueFilterDto> selectByUser(String user);
 
-  List<IssueFilterDto> selectByUserWithOnlyFavoriteFilters(String user);
+  List<IssueFilterDto> selectFavoriteFiltersByUser(String user);
 
-  List<IssueFilterDto> selectSharedWithoutUserFilters(String user);
-
-  @CheckForNull
-  IssueFilterDto selectSharedWithoutUserFiltersByName(@Param("name") String name, @Param("userLogin") String userLogin, @Nullable @Param("existingId") Long existingId);
+  List<IssueFilterDto> selectSharedFilters();
 
   void insert(IssueFilterDto filter);
 
index f8aaa53f94511d61d8b764e1f2227a68b95474b9..15d9b8d92638b88739dd81a65dab363164a5992c 100644 (file)
     </where>
   </select>
 
-  <select id="selectByIssueFilterId" parameterType="long" resultType="issueFilterFavourite">
+  <select id="selectByFilterId" parameterType="long" resultType="issueFilterFavourite">
     select <include refid="issueFilterFavouriteColumns"/>
     from issue_filter_favourites filter_favourites
     <where>
-      filter_favourites.issue_filter_id=#{issueFilterId}
-      and filter_favourites.user_login=#{userLogin}
+      filter_favourites.issue_filter_id=#{filterId}
     </where>
   </select>
 
@@ -37,7 +36,7 @@
     delete from issue_filter_favourites where id=#{id}
   </delete>
 
-  <delete id="deleteByIssueFilterId" parameterType="int">
+  <delete id="deleteByFilterId" parameterType="int">
     delete from issue_filter_favourites where issue_filter_id=#{issueFilterId}
   </delete>
 
index 21a2c87cfee88a48beea9f82b3534c259369535b..954c7af61f07ed8c806d6a160ce5708db6572232 100644 (file)
     </where>
   </select>
 
-  <select id="selectByNameAndUser" parameterType="map" resultType="IssueFilter">
-    select <include refid="issueFilterColumns"/>
-    from issue_filters filters
-    <where>
-      and filters.name=#{name}
-      and filters.user_login=#{userLogin}
-      <if test="existingId != null">
-        and filters.id&lt;&gt;#{existingId}
-      </if>
-    </where>
-  </select>
-
   <select id="selectByUser" parameterType="String" resultType="IssueFilter">
     select <include refid="issueFilterColumns"/>
     from issue_filters filters
@@ -43,7 +31,7 @@
     </where>
   </select>
 
-  <select id="selectByUserWithOnlyFavoriteFilters" parameterType="String" resultType="IssueFilter">
+  <select id="selectFavoriteFiltersByUser" parameterType="String" resultType="IssueFilter">
     select <include refid="issueFilterColumns"/>
     from issue_filters filters
     inner join issue_filter_favourites fav on fav.issue_filter_id = filters.id
     </where>
   </select>
 
-  <select id="selectSharedWithoutUserFilters" parameterType="String" resultType="IssueFilter">
-    select <include refid="issueFilterColumns"/>
-    from issue_filters filters
-    <where>
-      filters.shared=${_true}
-      and filters.user_login&lt;&gt;#{userLogin}
-    </where>
-  </select>
-
-  <select id="selectSharedWithoutUserFiltersByName" parameterType="String" resultType="IssueFilter">
+  <select id="selectSharedFilters" parameterType="String" resultType="IssueFilter">
     select <include refid="issueFilterColumns"/>
     from issue_filters filters
     <where>
-      filters.shared=${_true}
-      and filters.user_login&lt;&gt;#{userLogin}
-      and filters.name=#{name}
-      <if test="existingId != null">
-        and filters.id&lt;&gt;#{existingId}
-      </if>
+      and filters.shared=${_true}
     </where>
   </select>
 
index fd3eb8e7e93d5f5c2d20b9f870322d9d406f832e..1061e5764a59039b58aac3773e045a34db468b4b 100644 (file)
@@ -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<IssueFilterDto> results = dao.selectByUserWithOnlyFavoriteFilters("michael");
+    List<IssueFilterDto> 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
index c41e1ab52a98d7f051587de757fccb5893f4c374..e1112415a42732f764ffc40cac9421142cc37e25 100644 (file)
@@ -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<IssueFilterFavouriteDto> 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 (file)
index 0dc50ff..0000000
+++ /dev/null
@@ -1,27 +0,0 @@
-<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>
index 22b0c9e86765deca9e7e9924aa6e79f54e145824..f52804e5da0b5ad57dd1e972add3cab715a8888a 100644 (file)
@@ -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<DefaultIssueFilter> createIssueFilterResult(Map<String, String> params, boolean showCheckId, boolean showCheckUser) {
+  Result<DefaultIssueFilter> createIssueFilterResult(Map<String, String> params, boolean checkId, boolean checkUser) {
     Result<DefaultIssueFilter> 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<DefaultIssueFilter> findSharedFiltersForCurrentUser() {
-    return issueFilterService.findSharedFilters(UserSession.get());
+    return issueFilterService.findSharedFiltersWithoutUserFilters(UserSession.get());
   }
 
   public List<DefaultIssueFilter> findFavouriteIssueFiltersForCurrentUser() {
index afdd7f6841ecd851260ff71332492d6072e45205..77568fbd46aeef757e2b352fd7c6aa516fcc6faf 100644 (file)
@@ -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<DefaultIssueFilter> 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<String, Object> 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<DefaultIssueFilter> findSharedFilters(UserSession userSession) {
-    return toIssueFilters(issueFilterDao.selectSharedWithoutUserFilters(getNotNullLogin(userSession)));
+  public List<DefaultIssueFilter> findSharedFiltersWithoutUserFilters(UserSession userSession) {
+    final String login = getLoggedLogin(userSession);
+    return toIssueFilters(newArrayList(Iterables.filter(selectSharedFilters(), new Predicate<IssueFilterDto>() {
+      @Override
+      public boolean apply(IssueFilterDto input) {
+        return !input.getUserLogin().equals(login);
+      }
+    })));
   }
 
   public List<DefaultIssueFilter> 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<String, Object> 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<IssueFilterDto> 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<IssueFilterDto> 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<IssueFilterFavouriteDto>() {
+      @Override
+      public boolean apply(IssueFilterFavouriteDto input) {
+        return input.getUserLogin().equals(user);
+      }
+    }, null);
+  }
+
+  private List<IssueFilterFavouriteDto> selectFavouriteFilters(Long filterId) {
+    return favouriteDao.selectByFilterId(filterId);
+  }
+
+  private List<IssueFilterDto> selectUserIssueFilters(String user){
+    return filterDao.selectByUser(user);
+  }
+
+  private List<IssueFilterDto> selectSharedFilters(){
+    return filterDao.selectSharedFilters();
+  }
+
+  @CheckForNull
+  private IssueFilterDto findFilterWithSameName(List<IssueFilterDto> dtos, final String name){
+    return Iterables.find(dtos, new Predicate<IssueFilterDto>() {
+      @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();
   }
index 296a64778c7abf3c032d4d80ab5bb66b1677089c..b9fe11d8b861d0240a663ab0f262affd211afb08 100644 (file)
@@ -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());
   }
 
index c3ed479cc3fc9a9d196bd3969d8d339ed351218a..75cb12460eaa83949eac57abd303356d7a85c75c 100644 (file)
@@ -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.<IssueFilterDto>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<DefaultIssueFilter> results = service.findSharedFilters(userSession);
+    List<DefaultIssueFilter> 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<DefaultIssueFilter> 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.<IssueFilterFavouriteDto>emptyList());
 
     ArgumentCaptor<IssueFilterFavouriteDto> 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.<IssueFilterFavouriteDto>emptyList());
 
     ArgumentCaptor<IssueFilterFavouriteDto> 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);