From b9e0daf97c2107da6575bb7c4977c35193952248 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Sat, 4 May 2013 10:34:31 +0200 Subject: [PATCH] SONAR-3755 refactor issue sorting --- .../core/issue/db/ChangeDtoConverter.java | 4 +++ .../org/sonar/core/issue/db/IssueDao.java | 7 ----- .../core/plugins/PluginClassloaders.java | 2 +- .../core/source/HtmlSourceDecorator.java | 2 +- .../org/sonar/core/issue/db/IssueMapper.xml | 8 +++--- .../org/sonar/core/issue/db/IssueDaoTest.java | 4 +-- .../java/org/sonar/api/issue/IssueQuery.java | 26 +++++++------------ .../main/java/org/sonar/api/rules/Rule.java | 4 +-- .../org/sonar/api/issue/IssueQueryTest.java | 16 ++++-------- .../sonar/server/issue/ServerIssueFinder.java | 4 ++- .../org/sonar/server/issue/WebIssuesApi.java | 6 +++-- .../sonar/server/platform/UserSession.java | 10 +++---- 12 files changed, 40 insertions(+), 53 deletions(-) diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/ChangeDtoConverter.java b/sonar-core/src/main/java/org/sonar/core/issue/db/ChangeDtoConverter.java index 23015627eb7..c5eaabf0b4d 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/ChangeDtoConverter.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/ChangeDtoConverter.java @@ -29,6 +29,10 @@ import java.util.List; class ChangeDtoConverter { + private ChangeDtoConverter() { + // only static methods + } + static final String TYPE_FIELD_CHANGE = "change"; static final String TYPE_COMMENT = "comment"; diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java index f28fb0ac60f..9e6bcf82f4c 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java @@ -43,13 +43,6 @@ public class IssueDao implements BatchComponent, ServerComponent { private final MyBatis mybatis; - private static final Map SORTS = ImmutableMap.of( - "created", "i.issue_creation_date", - "updated", "i.issue_update_date", - "closed", "i.issue_close_date", - "assignee", "i.assignee" - ); - public IssueDao(MyBatis mybatis) { this.mybatis = mybatis; } diff --git a/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java b/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java index 91394566a71..8999a8f6f05 100644 --- a/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java +++ b/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java @@ -119,7 +119,7 @@ public class PluginClassloaders { } else { parent = new ResourcesClassloader(resources, baseClassloader); } - final ClassRealm realm; + ClassRealm realm; if (plugin.isUseChildFirstClassLoader()) { ClassRealm parentRealm = world.newRealm(plugin.getKey() + "-parent", parent); realm = parentRealm.createChildRealm(plugin.getKey()); diff --git a/sonar-core/src/main/java/org/sonar/core/source/HtmlSourceDecorator.java b/sonar-core/src/main/java/org/sonar/core/source/HtmlSourceDecorator.java index 16df96a3010..eff897901d1 100644 --- a/sonar-core/src/main/java/org/sonar/core/source/HtmlSourceDecorator.java +++ b/sonar-core/src/main/java/org/sonar/core/source/HtmlSourceDecorator.java @@ -45,7 +45,7 @@ public class HtmlSourceDecorator implements ServerComponent { } @VisibleForTesting - protected HtmlSourceDecorator(SnapshotSourceDao snapshotSourceDao, SnapshotDataDao snapshotDataDao) { + HtmlSourceDecorator(SnapshotSourceDao snapshotSourceDao, SnapshotDataDao snapshotDataDao) { this.snapshotSourceDao = snapshotSourceDao; this.snapshotDataDao= snapshotDataDao; } 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 4f0c14f67d2..7b0ea92b70c 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 @@ -215,16 +215,16 @@ order by - + i.issue_creation_date - + i.issue_update_date - + i.issue_close_date - + i.assignee_login diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java index 7592dae2f08..7a9461e2a0d 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java @@ -173,8 +173,8 @@ public class IssueDaoTest extends AbstractDaoTestCase { public void should_select_sort_by_assignee() { setupData("shared", "should_select_returned_sorted_result"); - IssueQuery query = IssueQuery.builder().sort("assignee").asc(true).build(); - List results = newArrayList(dao.select(query)); + IssueQuery query = IssueQuery.builder().sort(IssueQuery.Sort.ASSIGNEE).asc(true).build(); + List < IssueDto > results = newArrayList(dao.select(query)); assertThat(results).hasSize(3); assertThat(results.get(0).getAssignee()).isEqualTo("arthur"); assertThat(results.get(1).getAssignee()).isEqualTo("henry"); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java index d3fedd09ac7..b80ea04b91e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java @@ -25,8 +25,6 @@ import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.sonar.api.rule.RuleKey; import javax.annotation.Nullable; - -import java.util.Arrays; import java.util.Collection; import java.util.Date; @@ -37,6 +35,10 @@ import java.util.Date; */ public class IssueQuery { + public static enum Sort { + CREATION_DATE, UPDATE_DATE, CLOSE_DATE, ASSIGNEE + } + private final Collection issueKeys; private final Collection severities; private final Collection statuses; @@ -49,7 +51,7 @@ public class IssueQuery { private final Boolean assigned; private final Date createdAfter; private final Date createdBefore; - private final String sort; + private final Sort sort; private final boolean asc; // max results per page @@ -125,7 +127,7 @@ public class IssueQuery { return createdBefore; } - public String sort() { + public Sort sort() { return sort; } @@ -156,10 +158,6 @@ public class IssueQuery { */ public static class Builder { - private enum Sort { - created, updated, closed, assignee - } - private static final int DEFAULT_PAGE_SIZE = 100; private static final int MAX_PAGE_SIZE = 1000; private static final int DEFAULT_PAGE_INDEX = 1; @@ -176,7 +174,7 @@ public class IssueQuery { private Boolean assigned = null; private Date createdAfter; private Date createdBefore; - private String sort; + private Sort sort; private boolean asc = false; private int pageSize = DEFAULT_PAGE_SIZE; private int pageIndex = DEFAULT_PAGE_INDEX; @@ -248,14 +246,8 @@ public class IssueQuery { return this; } - public Builder sort(@Nullable String sort) { - if (sort != null) { - try { - this.sort = Sort.valueOf(sort).name(); - } catch (IllegalArgumentException e){ - throw new IllegalArgumentException("Sort should contain only : " + Arrays.toString(Sort.values()), e); - } - } + public Builder sort(@Nullable Sort sort) { + this.sort = sort; return this; } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java index 3a5cc176be2..7bc4e18671c 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java @@ -422,8 +422,8 @@ public final class Rule { /** * @since 3.6 */ - public Rule setCreatedAt(Date created_at) { - this.createdAt = created_at; + public Rule setCreatedAt(Date d) { + this.createdAt = d; return this; } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java index 5a9d955bac1..c7646729b99 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java @@ -46,7 +46,7 @@ public class IssueQueryTest { .assigned(true) .createdAfter(new Date()) .createdBefore(new Date()) - .sort("assignee") + .sort(IssueQuery.Sort.ASSIGNEE) .pageSize(10) .pageIndex(2) .build(); @@ -62,7 +62,7 @@ public class IssueQueryTest { assertThat(query.rules()).containsOnly(RuleKey.of("squid", "AvoidCycle")); assertThat(query.createdAfter()).isNotNull(); assertThat(query.createdBefore()).isNotNull(); - assertThat(query.sort()).isEqualTo("assignee"); + assertThat(query.sort()).isEqualTo(IssueQuery.Sort.ASSIGNEE); assertThat(query.pageSize()).isEqualTo(10); assertThat(query.pageIndex()).isEqualTo(2); } @@ -104,14 +104,8 @@ public class IssueQueryTest { } @Test - public void should_validate_sort() throws Exception { - try { - IssueQuery.builder() - .sort("INVALID SORT") - .build(); - fail(); - } catch (Exception e) { - assertThat(e).hasMessage("Sort should contain only : [created, updated, closed, assignee]").isInstanceOf(IllegalArgumentException.class); - } + public void should_accept_null_sort() throws Exception { + IssueQuery query = IssueQuery.builder().sort(null).build(); + assertThat(query.sort()).isNull(); } } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java index 36d0985bc22..41e95dabf5c 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueFinder.java @@ -64,7 +64,9 @@ public class ServerIssueFinder implements IssueFinder { private final ResourceDao resourceDao; private final ActionPlanIssueDao actionPlanIssueDao; - public ServerIssueFinder(MyBatis myBatis, IssueDao issueDao, AuthorizationDao authorizationDao, DefaultRuleFinder ruleFinder, ResourceDao resourceDao, ActionPlanIssueDao actionPlanIssueDao) { + public ServerIssueFinder(MyBatis myBatis, IssueDao issueDao, AuthorizationDao authorizationDao, + DefaultRuleFinder ruleFinder, ResourceDao resourceDao, + ActionPlanIssueDao actionPlanIssueDao) { this.myBatis = myBatis; this.issueDao = issueDao; this.authorizationDao = authorizationDao; diff --git a/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesApi.java b/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesApi.java index 35d9f60a4b5..bb1a29425e2 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesApi.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesApi.java @@ -27,14 +27,12 @@ import com.google.common.primitives.Ints; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.WebIssues; -import org.sonar.api.issue.WebIssues; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.DateUtils; import org.sonar.api.web.UserRole; import org.sonar.server.platform.UserSession; import javax.annotation.Nullable; - import java.util.Collection; import java.util.Date; import java.util.List; @@ -78,6 +76,10 @@ public class WebIssuesApi implements WebIssues { builder.createdBefore(toDate(props.get("createdBefore"))); builder.pageSize(toInteger(props.get("pageSize"))); builder.pageIndex(toInteger(props.get("pageIndex"))); + String sort = (String) props.get("sort"); + if (sort != null) { + builder.sort(IssueQuery.Sort.valueOf(sort)); + } return builder.build(); } diff --git a/sonar-server/src/main/java/org/sonar/server/platform/UserSession.java b/sonar-server/src/main/java/org/sonar/server/platform/UserSession.java index 0639e884117..c81409d11cd 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/UserSession.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/UserSession.java @@ -28,7 +28,7 @@ import java.util.Locale; public class UserSession { - private static final ThreadLocal threadLocal = new ThreadLocal(); + private static final ThreadLocal THREAD_LOCAL = new ThreadLocal(); private static final UserSession DEFAULT_ANONYMOUS = new UserSession(null, null, Locale.ENGLISH); private final Integer userId; @@ -63,11 +63,11 @@ public class UserSession { * @return never null */ public static UserSession get() { - return Objects.firstNonNull(threadLocal.get(), DEFAULT_ANONYMOUS); + return Objects.firstNonNull(THREAD_LOCAL.get(), DEFAULT_ANONYMOUS); } public static void set(@Nullable UserSession session) { - threadLocal.set(session); + THREAD_LOCAL.set(session); } public static void setSession(@Nullable Integer userId, @Nullable String login, @Nullable String localeRubyKey) { @@ -75,11 +75,11 @@ public class UserSession { } public static void remove() { - threadLocal.remove(); + THREAD_LOCAL.remove(); } static boolean hasSession() { - return threadLocal.get() != null; + return THREAD_LOCAL.get() != null; } } -- 2.39.5