From 5765e0955d1516e035e7a970be9b992d9a90b270 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Sun, 12 May 2013 21:29:40 +0200 Subject: [PATCH] SONAR-3755 refactor comments --- .../sonar/core/issue/DefaultIssueComment.java | 13 ++- .../org/sonar/core/issue/IssueUpdater.java | 2 +- .../core/issue/db/ChangeDtoConverter.java | 5 +- .../sonar/core/issue/db/IssueChangeDao.java | 47 +++++--- .../core/issue/db/IssueChangeMapper.java | 9 +- .../org/sonar/core/issue/db/IssueDao.java | 26 +++-- .../org/sonar/core/issue/db/IssueMapper.java | 11 +- .../org/sonar/core/issue/db/IssueStorage.java | 2 +- .../org/sonar/core/persistence/MyBatis.java | 3 +- .../sonar/core/issue/db/IssueChangeMapper.xml | 18 +++- .../org/sonar/core/issue/db/IssueMapper.xml | 82 ++++++++------ .../core/issue/db/ChangeDtoConverterTest.java | 4 +- .../core/issue/db/IssueChangeDaoTest.java | 74 ++++++++++--- .../core/issue/db/IssueChangeMapperTest.java | 8 +- .../org/sonar/core/issue/db/IssueDaoTest.java | 18 +++- .../sonar/core/issue/db/IssueStorageTest.java | 4 +- .../db/IssueChangeDaoTest/delete-result.xml | 39 +++++++ .../issue/db/IssueChangeDaoTest/delete.xml | 35 ++++++ .../db/IssueChangeDaoTest/update-result.xml | 35 ++++++ .../issue/db/IssueChangeDaoTest/update.xml | 35 ++++++ ...t-result.xml => insert_comment-result.xml} | 0 ...Diff-result.xml => insert_diff-result.xml} | 0 .../db/IssueDaoTest/selectByChangeKey.xml | 76 +++++++++++++ ...should_select_issue_and_component_ids.xml} | 0 .../main/java/org/sonar/api/issue/Issue.java | 5 +- .../java/org/sonar/api/issue/IssueQuery.java | 10 +- .../server/issue/ServerIssueActions.java | 23 ++-- .../sonar/server/issue/ServerIssueFinder.java | 21 ++-- .../sonar/server/issue/WebIssuesInternal.java | 18 ++-- .../app/controllers/api/issues_controller.rb | 101 +++++++++++------- .../server/issue/ServerIssueFinderTest.java | 18 ++-- .../wsclient/issue/DefaultIssueClient.java | 13 ++- .../java/org/sonar/wsclient/issue/Issue.java | 23 ++-- .../org/sonar/wsclient/issue/IssueClient.java | 2 + .../sonar/wsclient/issue/IssueComment.java | 52 +++++++++ .../org/sonar/wsclient/issue/IssueParser.java | 3 + .../java/org/sonar/wsclient/issue/Issues.java | 4 + .../sonar/wsclient/issue/IssueParserTest.java | 25 +++++ .../IssueParserTest/issue-with-comments.json | 41 +++++++ 39 files changed, 727 insertions(+), 178 deletions(-) create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete-result.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update-result.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update.xml rename sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/{testInsertComment-result.xml => insert_comment-result.xml} (100%) rename sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/{testInsertDiff-result.xml => insert_diff-result.xml} (100%) create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/selectByChangeKey.xml rename sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/{should_select_issue_ids_and_components_ids.xml => should_select_issue_and_component_ids.xml} (100%) create mode 100644 sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueComment.java create mode 100644 sonar-ws-client/src/test/resources/org/sonar/wsclient/issue/IssueParserTest/issue-with-comments.json diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java index c248c2097a8..1a5e1f29b99 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueComment.java @@ -29,6 +29,7 @@ import java.util.UUID; public class DefaultIssueComment implements Serializable, IssueComment { + private String issueKey; private String userLogin; private Date createdAt, updatedAt; private String key; @@ -45,6 +46,15 @@ public class DefaultIssueComment implements Serializable, IssueComment { return this; } + public String issueKey() { + return issueKey; + } + + public DefaultIssueComment setIssueKey(String s) { + this.issueKey = s; + return this; + } + @Override public String key() { return key; @@ -98,8 +108,9 @@ public class DefaultIssueComment implements Serializable, IssueComment { return this; } - public static DefaultIssueComment create(@Nullable String login, String text) { + public static DefaultIssueComment create(String issueKey, @Nullable String login, String text) { DefaultIssueComment comment = new DefaultIssueComment(); + comment.setIssueKey(issueKey); comment.setKey(UUID.randomUUID().toString()); Date now = new Date(); comment.setUserLogin(login); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java index f42780fe36e..90da11f6076 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java @@ -100,7 +100,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { } public void addComment(DefaultIssue issue, String text, IssueChangeContext context) { - issue.addComment(DefaultIssueComment.create(context.login(), text)); + issue.addComment(DefaultIssueComment.create(issue.key(), context.login(), text)); } public void setCloseDate(DefaultIssue issue, @Nullable Date d) { 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 d2cfeefdc8b..19b9e9cffa9 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 @@ -33,8 +33,8 @@ class ChangeDtoConverter { static final String TYPE_FIELD_CHANGE = "diff"; static final String TYPE_COMMENT = "comment"; - static IssueChangeDto commentToDto(String issueKey, DefaultIssueComment comment) { - IssueChangeDto dto = newDto(issueKey); + static IssueChangeDto commentToDto(DefaultIssueComment comment) { + IssueChangeDto dto = newDto(comment.issueKey()); dto.setKey(comment.key()); dto.setChangeType(TYPE_COMMENT); dto.setChangeData(comment.text()); @@ -68,6 +68,7 @@ class ChangeDtoConverter { .setCreatedAt(dto.getCreatedAt()) .setUpdatedAt(dto.getUpdatedAt()) .setUserLogin(dto.getUserLogin()) + .setIssueKey(dto.getIssueKey()) .setNew(false); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java index b355ac4ebc0..7de80f403c7 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java @@ -20,11 +20,12 @@ package org.sonar.core.issue.db; +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; import org.sonar.core.issue.DefaultIssueComment; -import org.sonar.core.issue.FieldDiffs; import org.sonar.core.persistence.MyBatis; import java.util.Collection; @@ -41,32 +42,44 @@ public class IssueChangeDao implements BatchComponent, ServerComponent { this.mybatis = mybatis; } - public DefaultIssueComment[] selectIssueComments(String issueKey) { - List dtos = selectByIssue(issueKey, ChangeDtoConverter.TYPE_COMMENT); - DefaultIssueComment[] result = new DefaultIssueComment[dtos.size()]; - for (int index = 0; index < dtos.size(); index++) { - result[index] = ChangeDtoConverter.dtoToComment(dtos.get(index)); + public List selectCommentsByIssues(SqlSession session, Collection issueKeys) { + return selectByIssuesAndType(session, issueKeys, ChangeDtoConverter.TYPE_COMMENT); + } + + List selectByIssuesAndType(SqlSession session, Collection issueKeys, String changeType) { + Preconditions.checkArgument(issueKeys.size() < 1000, "Number of issue keys is greater than or equal 1000"); + List result = Lists.newArrayList(); + if (!issueKeys.isEmpty()) { + IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + List dtos = mapper.selectByIssuesAndType(issueKeys, changeType); + for (IssueChangeDto dto : dtos) { + result.add(ChangeDtoConverter.dtoToComment(dto)); + } } return result; } - public FieldDiffs[] selectIssueChanges(String issueKey) { - List dtos = selectByIssue(issueKey, ChangeDtoConverter.TYPE_FIELD_CHANGE); - FieldDiffs[] result = new FieldDiffs[dtos.size()]; - for (int index = 0; index < dtos.size(); index++) { - result[index] = ChangeDtoConverter.dtoToChange(dtos.get(index)); + public boolean delete(String key) { + SqlSession session = mybatis.openSession(); + try { + IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + int count = mapper.delete(key); + session.commit(); + return count==1; + + } finally { + MyBatis.closeQuietly(session); } - return result; } - /** - * Issue changes by chronological date of creation - */ - private List selectByIssue(String issueKey, String changeType) { + public boolean update(IssueChangeDto change) { SqlSession session = mybatis.openSession(); try { IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); - return mapper.selectByIssueAndType(issueKey, changeType); + int count = mapper.update(change); + session.commit(); + return count==1; + } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java index 9c049b38b65..2f797a13801 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeMapper.java @@ -22,6 +22,7 @@ package org.sonar.core.issue.db; import org.apache.ibatis.annotations.Param; +import java.util.Collection; import java.util.List; /** @@ -31,9 +32,13 @@ public interface IssueChangeMapper { void insert(IssueChangeDto dto); + int delete(String key); + + int update(IssueChangeDto change); + /** * Issue changes by chronological date of creation */ - List selectByIssueAndType(@Param("issueKey") String issueKey, @Param("changeType") String changeType); - + List selectByIssuesAndType(@Param("issueKeys") Collection issueKeys, + @Param("changeType") String changeType); } 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 9e6bcf82f4c..b280000920e 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 @@ -50,7 +50,18 @@ public class IssueDao implements BatchComponent, ServerComponent { public IssueDto selectByKey(String key) { SqlSession session = mybatis.openSession(); try { - return session.selectOne("org.sonar.core.issue.db.IssueMapper.selectByKey", key); + IssueMapper mapper = session.getMapper(IssueMapper.class); + return mapper.selectByKey(key); + } finally { + MyBatis.closeQuietly(session); + } + } + + public IssueDto selectByChangeKey(String commentKey) { + SqlSession session = mybatis.openSession(); + try { + IssueMapper mapper = session.getMapper(IssueMapper.class); + return mapper.selectByChangeKey(commentKey); } finally { MyBatis.closeQuietly(session); } @@ -69,17 +80,18 @@ public class IssueDao implements BatchComponent, ServerComponent { public List select(IssueQuery query) { SqlSession session = mybatis.openSession(); try { - return session.selectList("org.sonar.core.issue.db.IssueMapper.select", query); + IssueMapper mapper = session.getMapper(IssueMapper.class); + return mapper.select(query); } finally { MyBatis.closeQuietly(session); } } @VisibleForTesting - List selectIssueIdsAndComponentsId(IssueQuery query) { + List selectIssueAndComponentIds(IssueQuery query) { SqlSession session = mybatis.openSession(); try { - return selectIssueIdsAndComponentsId(query, session); + return selectIssueAndComponentIds(query, session); } finally { MyBatis.closeQuietly(session); } @@ -88,9 +100,9 @@ public class IssueDao implements BatchComponent, ServerComponent { /** * The returned IssueDto list contains only the issue id and the resource id */ - public List selectIssueIdsAndComponentsId(IssueQuery query, SqlSession session) { - // TODO support ordering - return session.selectList("org.sonar.core.issue.db.IssueMapper.selectIssueIdsAndComponentsId", query); + public List selectIssueAndComponentIds(IssueQuery query, SqlSession session) { + IssueMapper mapper = session.getMapper(IssueMapper.class); + return mapper.selectIssueAndComponentIds(query); } Collection selectByIds(Collection ids) { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java index 4e65007e824..1073d3a2766 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java @@ -19,9 +19,18 @@ */ package org.sonar.core.issue.db; +import org.sonar.api.issue.IssueQuery; + +import java.util.List; + public interface IssueMapper { + IssueDto selectByKey(String key); + IssueDto selectByChangeKey(String changeKey); + List selectIssueAndComponentIds(IssueQuery query); + + List select(IssueQuery query); void insert(IssueDto issue); - int update(IssueDto issue); + int update(IssueDto issue); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java index 7bf78f6af1d..af308fb0029 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java @@ -81,7 +81,7 @@ public abstract class IssueStorage { for (IssueComment comment : issue.comments()) { DefaultIssueComment c = (DefaultIssueComment) comment; if (c.isNew()) { - IssueChangeDto changeDto = ChangeDtoConverter.commentToDto(issue.key(), c); + IssueChangeDto changeDto = ChangeDtoConverter.commentToDto(c); mapper.insert(changeDto); } } diff --git a/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java b/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java index be615a6eadc..e5dfb458eb2 100644 --- a/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java +++ b/sonar-core/src/main/java/org/sonar/core/persistence/MyBatis.java @@ -126,14 +126,13 @@ public class MyBatis implements BatchComponent, ServerComponent { loadAlias(conf, "ActionPlanStats", ActionPlanStatsDto.class); Class[] mappers = {ActiveDashboardMapper.class, AuthorMapper.class, DashboardMapper.class, - DependencyMapper.class, DuplicationMapper.class, GraphDtoMapper.class, IssueChangeMapper.class, LoadedTemplateMapper.class, + DependencyMapper.class, DuplicationMapper.class, GraphDtoMapper.class, IssueMapper.class, IssueChangeMapper.class, LoadedTemplateMapper.class, MeasureFilterMapper.class, PropertiesMapper.class, PurgeMapper.class, ResourceKeyUpdaterMapper.class, ResourceIndexerMapper.class, ResourceMapper.class, ResourceSnapshotMapper.class, ReviewCommentMapper.class, ReviewMapper.class, RoleMapper.class, RuleMapper.class, SchemaMigrationMapper.class, SemaphoreMapper.class, UserMapper.class, WidgetMapper.class, WidgetPropertyMapper.class, MeasureMapper.class, SnapshotDataMapper.class, SnapshotSourceMapper.class, ActionPlanMapper.class, ActionPlanStatsMapper.class }; loadMappers(conf, mappers); - loadMapper(conf, "org.sonar.core.issue.db.IssueMapper"); loadMapper(conf, "org.sonar.core.user.AuthorizationMapper"); configureLogback(mappers); diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml index c54639a2c2d..5c2a92b2cab 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml @@ -41,7 +41,8 @@ - + select issue_changes_seq.NEXTVAL from DUAL @@ -49,13 +50,22 @@ VALUES (#{id}, #{kee}, #{issueKey}, #{userLogin}, #{changeType}, #{changeData}, #{createdAt}, #{updatedAt}) - select from issue_changes c - where c.issue_key=#{issueKey} and c.change_type=#{changeType} + where c.change_type=#{changeType} and c.issue_key in ( + #{key} + ) order by c.created_at - 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 5c3a81d1fc0..92e325ad2d2 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 @@ -34,50 +34,55 @@ - INSERT INTO issues (kee, resource_id, rule_id, action_plan_key, severity, manual_severity, manual_issue, description, line, effort_to_fix, status, + INSERT INTO issues (kee, resource_id, rule_id, action_plan_key, severity, manual_severity, manual_issue, + description, line, effort_to_fix, status, resolution, checksum, user_login, assignee_login, author_login, attributes, issue_creation_date, issue_update_date, issue_close_date, created_at, updated_at) - VALUES (#{kee}, #{resourceId}, #{ruleId}, #{actionPlanKey}, #{severity}, #{manualSeverity}, #{manualIssue}, #{description}, #{line}, #{effortToFix}, #{status}, + VALUES (#{kee}, #{resourceId}, #{ruleId}, #{actionPlanKey}, #{severity}, #{manualSeverity}, #{manualIssue}, + #{description}, #{line}, #{effortToFix}, #{status}, #{resolution}, #{checksum}, #{userLogin}, #{assignee}, #{authorLogin}, #{attributes}, #{issueCreationDate}, #{issueUpdateDate}, #{issueCloseDate}, #{createdAt}, #{updatedAt}) - + select issues_seq.NEXTVAL from DUAL - INSERT INTO issues (id, kee, resource_id, rule_id, action_plan_key, severity, manual_severity, manual_issue, description, line, effort_to_fix, status, + INSERT INTO issues (id, kee, resource_id, rule_id, action_plan_key, severity, manual_severity, manual_issue, + description, line, effort_to_fix, status, resolution, checksum, user_login, assignee_login, author_login, attributes, issue_creation_date, issue_update_date, issue_close_date, created_at, updated_at) - VALUES (#{id}, #{kee}, #{resourceId}, #{ruleId}, #{actionPlanKey}, #{severity}, #{manualSeverity}, #{manualIssue}, #{description}, #{line}, #{effortToFix}, #{status}, + VALUES (#{id}, #{kee}, #{resourceId}, #{ruleId}, #{actionPlanKey}, #{severity}, #{manualSeverity}, #{manualIssue}, + #{description}, #{line}, #{effortToFix}, #{status}, #{resolution}, #{checksum}, #{userLogin}, #{assignee}, #{authorLogin}, #{attributes}, #{issueCreationDate}, #{issueUpdateDate}, #{issueCloseDate}, #{createdAt}, #{updatedAt}) update issues set - resource_id=#{resourceId}, - rule_id=#{ruleId}, - action_plan_key=#{actionPlanKey}, - severity=#{severity}, - manual_severity=#{manualSeverity}, - manual_issue=#{manualIssue}, - description=#{description}, - line=#{line}, - effort_to_fix=#{effortToFix}, - status=#{status}, - resolution=#{resolution}, - checksum=#{checksum}, - user_login=#{userLogin}, - assignee_login=#{assignee}, - author_login=#{authorLogin}, - attributes=#{attributes}, - issue_creation_date=#{issueCreationDate}, - issue_update_date=#{issueUpdateDate}, - issue_close_date=#{issueCloseDate}, - created_at=#{createdAt}, - updated_at=#{updatedAt} + resource_id=#{resourceId}, + rule_id=#{ruleId}, + action_plan_key=#{actionPlanKey}, + severity=#{severity}, + manual_severity=#{manualSeverity}, + manual_issue=#{manualIssue}, + description=#{description}, + line=#{line}, + effort_to_fix=#{effortToFix}, + status=#{status}, + resolution=#{resolution}, + checksum=#{checksum}, + user_login=#{userLogin}, + assignee_login=#{assignee}, + author_login=#{authorLogin}, + attributes=#{attributes}, + issue_creation_date=#{issueCreationDate}, + issue_update_date=#{issueUpdateDate}, + issue_close_date=#{issueCloseDate}, + created_at=#{createdAt}, + updated_at=#{updatedAt} where kee = #{kee} @@ -88,14 +93,23 @@ where i.kee=#{kee} and i.rule_id=r.id and p.id=i.resource_id + + - select i.id, i.resource_id as resourceId @@ -213,11 +227,13 @@ - and (r.plugin_name=#{rule.repository} and r.plugin_rule_key=#{rule.rule}) + and ( + r.plugin_name=#{rule.repository} and r.plugin_rule_key=#{rule.rule}) and i.action_plan_key in - #{action_plan} + + #{action_plan} diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/ChangeDtoConverterTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/ChangeDtoConverterTest.java index e7f5ff2b26c..8b0e67347d9 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/ChangeDtoConverterTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/ChangeDtoConverterTest.java @@ -28,9 +28,9 @@ import static org.fest.assertions.Assertions.assertThat; public class ChangeDtoConverterTest { @Test public void testToChangeDtos() throws Exception { - DefaultIssueComment comment = DefaultIssueComment.create("emmerik", "the comment"); + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); - IssueChangeDto dto = ChangeDtoConverter.commentToDto("ABCDE", comment); + IssueChangeDto dto = ChangeDtoConverter.commentToDto(comment); assertThat(dto.getChangeData()).isEqualTo("the comment"); assertThat(dto.getChangeType()).isEqualTo("comment"); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java index 426ea6b2e14..3d3c641ba8f 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java @@ -19,17 +19,19 @@ */ package org.sonar.core.issue.db; +import org.apache.ibatis.session.SqlSession; import org.junit.Before; import org.junit.Test; import org.sonar.api.utils.DateUtils; import org.sonar.core.issue.DefaultIssueComment; -import org.sonar.core.issue.FieldDiffs; import org.sonar.core.persistence.AbstractDaoTestCase; import java.util.Arrays; -import java.util.Date; +import java.util.Collections; +import java.util.List; import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; public class IssueChangeDaoTest extends AbstractDaoTestCase { @@ -41,31 +43,77 @@ public class IssueChangeDaoTest extends AbstractDaoTestCase { } @Test - public void should_select_issue_comments() { + public void selectCommentsByIssues() { setupData("shared"); - DefaultIssueComment[] comments = dao.selectIssueComments("1000"); + SqlSession session = getMyBatis().openSession(); + List comments = dao.selectCommentsByIssues(session, Arrays.asList("1000")); + session.close(); assertThat(comments).hasSize(2); // chronological order - DefaultIssueComment first = comments[0]; + DefaultIssueComment first = comments.get(0); assertThat(first.text()).isEqualTo("old comment"); - DefaultIssueComment second = comments[1]; + DefaultIssueComment second = comments.get(1); assertThat(second.userLogin()).isEqualTo("arthur"); assertThat(second.key()).isEqualTo("FGHIJ"); assertThat(second.text()).isEqualTo("recent comment"); } @Test - public void should_select_issue_changes() { - setupData("shared"); + public void selectCommentsByIssues_empty_input() { + // no need to connect to db + SqlSession session = mock(SqlSession.class); + List comments = dao.selectCommentsByIssues(session, Collections.emptyList()); + + assertThat(comments).isEmpty(); + } + + @Test + public void delete() { + setupData("delete"); + + assertThat(dao.delete("COMMENT-2")).isTrue(); + + checkTable("delete", "issue_changes"); + } + + @Test + public void delete_unknown_key() { + setupData("delete"); + + assertThat(dao.delete("UNKNOWN")).isFalse(); + } + + @Test + public void update() { + setupData("update"); + + IssueChangeDto change = new IssueChangeDto(); + change.setKey("COMMENT-2"); + + // Only the following fields can be updated: + change.setChangeData("new comment"); + change.setUpdatedAt(DateUtils.parseDate("2013-06-30")); + + assertThat(dao.update(change)).isTrue(); + + checkTable("update", "issue_changes"); + } + + @Test + public void update_unknown_key() { + setupData("update"); + + IssueChangeDto change = new IssueChangeDto(); + change.setKey("UNKNOWN"); + + // Only the following fields can be updated: + change.setChangeData("new comment"); + change.setUpdatedAt(DateUtils.parseDate("2013-06-30")); - FieldDiffs[] ordered = dao.selectIssueChanges("1000"); - assertThat(ordered).hasSize(1); - FieldDiffs.Diff severityDiff = ordered[0].get("severity"); - assertThat(severityDiff.oldValue()).isEqualTo("MAJOR"); - assertThat(severityDiff.newValue()).isEqualTo("BLOCKER"); + assertThat(dao.update(change)).isFalse(); } } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeMapperTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeMapperTest.java index 9c320bf12cd..316c041879a 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeMapperTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeMapperTest.java @@ -38,7 +38,7 @@ public class IssueChangeMapperTest extends AbstractDaoTestCase { } @Test - public void testInsertDiff() throws Exception { + public void insert_diff() throws Exception { IssueChangeDto dto = new IssueChangeDto(); dto.setKey(null /* no key on field changes */); dto.setUserLogin("emmerik"); @@ -51,11 +51,11 @@ public class IssueChangeMapperTest extends AbstractDaoTestCase { mapper.insert(dto); session.commit(); - checkTables("testInsertDiff", new String[]{"id"}, "issue_changes"); + checkTables("insert_diff", new String[]{"id"}, "issue_changes"); } @Test - public void testInsertComment() throws Exception { + public void insert_comment() throws Exception { IssueChangeDto dto = new IssueChangeDto(); dto.setKey("COMMENT-1234"); dto.setUserLogin("emmerik"); @@ -68,6 +68,6 @@ public class IssueChangeMapperTest extends AbstractDaoTestCase { mapper.insert(dto); session.commit(); - checkTables("testInsertComment", new String[]{"id"}, "issue_changes"); + checkTables("insert_comment", new String[]{"id"}, "issue_changes"); } } 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 0d35a9b5327..4238afe1fc9 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 @@ -204,7 +204,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { setupData("shared", "should_select_returned_sorted_result"); IssueQuery query = IssueQuery.builder().sort(IssueQuery.Sort.ASSIGNEE).asc(true).build(); - List < IssueDto > results = newArrayList(dao.select(query)); + List results = newArrayList(dao.select(query)); assertThat(results).hasSize(3); assertThat(results.get(0).getAssignee()).isEqualTo("arthur"); assertThat(results.get(1).getAssignee()).isEqualTo("henry"); @@ -212,11 +212,11 @@ public class IssueDaoTest extends AbstractDaoTestCase { } @Test - public void should_select_issue_ids_and_components_ids() { - setupData("shared", "should_select_issue_ids_and_components_ids"); + public void should_select_issue_and_component_ids() { + setupData("shared", "should_select_issue_and_component_ids"); IssueQuery query = IssueQuery.builder().build(); - List results = dao.selectIssueIdsAndComponentsId(query); + List results = dao.selectIssueAndComponentIds(query); assertThat(results).hasSize(3); } @@ -241,4 +241,14 @@ public class IssueDaoTest extends AbstractDaoTestCase { assertThat(results).hasSize(3); } + @Test + public void selectByChangeKey() throws Exception { + setupData("shared", "selectByChangeKey"); + IssueDto issue = dao.selectByChangeKey("COMMENT-20"); + assertThat(issue.getKee()).isEqualTo("ISSUE-2"); + + issue = dao.selectByChangeKey("COMMENT-UNKNOWN"); + assertThat(issue).isNull(); + } + } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java index 1aa38f19657..1668bcfcde6 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java @@ -42,7 +42,7 @@ public class IssueStorageTest extends AbstractDaoTestCase { public void should_insert_new_issues() throws Exception { FakeSaver saver = new FakeSaver(getMyBatis(), new FakeRuleFinder()); - DefaultIssueComment comment = DefaultIssueComment.create("emmerik", "the comment"); + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); // override generated key comment.setKey("FGHIJ"); @@ -71,7 +71,7 @@ public class IssueStorageTest extends AbstractDaoTestCase { FakeSaver saver = new FakeSaver(getMyBatis(), new FakeRuleFinder()); - DefaultIssueComment comment = DefaultIssueComment.create("emmerik", "the comment"); + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); // override generated key comment.setKey("FGHIJ"); diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete-result.xml new file mode 100644 index 00000000000..38915608de0 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete-result.xml @@ -0,0 +1,39 @@ + + + + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete.xml new file mode 100644 index 00000000000..91b53b495ee --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/delete.xml @@ -0,0 +1,35 @@ + + + + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update-result.xml new file mode 100644 index 00000000000..02577e213df --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update-result.xml @@ -0,0 +1,35 @@ + + + + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update.xml new file mode 100644 index 00000000000..f7dfdc84832 --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/update.xml @@ -0,0 +1,35 @@ + + + + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/testInsertComment-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/insert_comment-result.xml similarity index 100% rename from sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/testInsertComment-result.xml rename to sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/insert_comment-result.xml diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/testInsertDiff-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/insert_diff-result.xml similarity index 100% rename from sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/testInsertDiff-result.xml rename to sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeMapperTest/insert_diff-result.xml diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/selectByChangeKey.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/selectByChangeKey.xml new file mode 100644 index 00000000000..a8da7191bdf --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/selectByChangeKey.xml @@ -0,0 +1,76 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/should_select_issue_ids_and_components_ids.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/should_select_issue_and_component_ids.xml similarity index 100% rename from sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/should_select_issue_ids_and_components_ids.xml rename to sonar-core/src/test/resources/org/sonar/core/issue/db/IssueDaoTest/should_select_issue_and_component_ids.xml diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java index 61bfe64f6b1..1def6c0099f 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java @@ -75,7 +75,7 @@ public interface Issue extends Serializable { String status(); /** - * The type of resolution. Return null if the issue is not resolved. + * The type of resolution, or null if the issue is not resolved. */ @CheckForNull String resolution(); @@ -106,5 +106,8 @@ public interface Issue extends Serializable { @CheckForNull String actionPlanKey(); + /** + * Non-null list of comments, ordered by chronological order + */ List comments(); } 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 cf28674d90d..87f0e462347 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,7 +25,6 @@ import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.sonar.api.rule.RuleKey; import javax.annotation.Nullable; - import java.util.Collection; import java.util.Date; @@ -174,6 +173,7 @@ public class IssueQuery { private static final int DEFAULT_PAGE_SIZE = 100; private static final int MAX_PAGE_SIZE = 1000; private static final int DEFAULT_PAGE_INDEX = 1; + private static final int MAX_ISSUE_KEYS = 1000; private Collection issueKeys; private Collection severities; @@ -286,8 +286,6 @@ public class IssueQuery { } public Builder pageSize(@Nullable Integer i) { - Preconditions.checkArgument(i == null || i > 0, "Page size must be greater than 0 (got " + i + ")"); - Preconditions.checkArgument(i == null || i < MAX_PAGE_SIZE, "Page size must be less than " + MAX_PAGE_SIZE + " (got " + i + ")"); this.pageSize = (i == null ? DEFAULT_PAGE_SIZE : i.intValue()); return this; } @@ -299,6 +297,12 @@ public class IssueQuery { } public IssueQuery build() { + Preconditions.checkArgument(pageSize > 0, "Page size must be greater than 0 (got " + pageSize + ")"); + Preconditions.checkArgument(pageSize < MAX_PAGE_SIZE, "Page size must be less than " + MAX_PAGE_SIZE + " (got " + pageSize + ")"); + if (issueKeys != null) { + Preconditions.checkArgument(issueKeys.size() < MAX_ISSUE_KEYS, "Number of issue keys must be less than " + MAX_ISSUE_KEYS + " (got " + issueKeys.size() + ")"); + } + return new IssueQuery(this); } } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueActions.java b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueActions.java index 1c46d4cf406..42e57e9afcd 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueActions.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueActions.java @@ -24,7 +24,10 @@ import org.sonar.api.ServerComponent; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueComment; import org.sonar.api.web.UserRole; -import org.sonar.core.issue.*; +import org.sonar.core.issue.ActionPlanManager; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; +import org.sonar.core.issue.IssueUpdater; import org.sonar.core.issue.db.IssueChangeDao; import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; @@ -122,23 +125,23 @@ public class ServerIssueActions implements ServerComponent { return issue; } - public IssueComment addComment(String issueKey, String comment, UserSession userSession) { + public IssueComment addComment(String issueKey, String text, UserSession userSession) { DefaultIssue issue = loadIssue(issueKey, userSession); IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login()); - issueUpdater.addComment(issue, comment, context); + issueUpdater.addComment(issue, text, context); issueStorage.save(issue); - return issue.comments().get(0); + return issue.comments().get(issue.comments().size() - 1); } - public DefaultIssueComment[] comments(String issueKey, UserSession userSession) { - // TODO verify authorization - return issueChangeDao.selectIssueComments(issueKey); + public IssueComment deleteComment(String key, UserSession userSession) { + // TODO + return null; } - public FieldDiffs[] changes(String issueKey, UserSession userSession) { - // TODO verify authorization - return issueChangeDao.selectIssueChanges(issueKey); + public IssueComment editComment(String key, String text, UserSession userSession) { + // TODO + return null; } public Issue create(DefaultIssue issue, UserSession userSession) { 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 94c567c863e..3e894a4c19d 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 @@ -33,6 +33,7 @@ import org.sonar.api.rules.Rule; import org.sonar.api.utils.Paging; import org.sonar.core.issue.ActionPlanManager; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.db.IssueChangeDao; import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; @@ -59,16 +60,20 @@ public class ServerIssueFinder implements IssueFinder { private static final Logger LOG = LoggerFactory.getLogger(ServerIssueFinder.class); private final MyBatis myBatis; private final IssueDao issueDao; + private final IssueChangeDao issueChangeDao; private final AuthorizationDao authorizationDao; private final DefaultRuleFinder ruleFinder; private final ResourceDao resourceDao; private final ActionPlanManager actionPlanManager; - public ServerIssueFinder(MyBatis myBatis, IssueDao issueDao, AuthorizationDao authorizationDao, + public ServerIssueFinder(MyBatis myBatis, + IssueDao issueDao, IssueChangeDao issueChangeDao, + AuthorizationDao authorizationDao, DefaultRuleFinder ruleFinder, ResourceDao resourceDao, ActionPlanManager actionPlanManager) { this.myBatis = myBatis; this.issueDao = issueDao; + this.issueChangeDao = issueChangeDao; this.authorizationDao = authorizationDao; this.ruleFinder = ruleFinder; this.resourceDao = resourceDao; @@ -79,24 +84,22 @@ public class ServerIssueFinder implements IssueFinder { LOG.debug("IssueQuery : {}", query); SqlSession sqlSession = myBatis.openSession(); try { - List allIssuesDto = issueDao.selectIssueIdsAndComponentsId(query, sqlSession); + List allIssuesDto = issueDao.selectIssueAndComponentIds(query, sqlSession); Set authorizedComponentIds = authorizationDao.keepAuthorizedComponentIds(extractResourceIds(allIssuesDto), currentUserId, role, sqlSession); List authorizedIssues = authorized(allIssuesDto, authorizedComponentIds); Paging paging = Paging.create(query.pageSize(), query.pageIndex(), authorizedIssues.size()); Set pagedAuthorizedIssueIds = pagedAuthorizedIssueIds(authorizedIssues, paging); Collection dtos = issueDao.selectByIds(pagedAuthorizedIssueIds, sqlSession); - Map issuesById = newHashMap(); + Map issuesByKey = newHashMap(); List issues = newArrayList(); - List issueIds = newArrayList(); Set ruleIds = Sets.newHashSet(); Set componentIds = Sets.newHashSet(); Set actionPlanKeys = Sets.newHashSet(); for (IssueDto dto : dtos) { if (authorizedComponentIds.contains(dto.getResourceId())) { DefaultIssue defaultIssue = dto.toDefaultIssue(); - issuesById.put(dto.getId(), defaultIssue); - issueIds.add(dto.getId()); + issuesByKey.put(dto.getKee(), defaultIssue); issues.add(defaultIssue); ruleIds.add(dto.getRuleId()); @@ -105,6 +108,12 @@ public class ServerIssueFinder implements IssueFinder { } } + List comments = issueChangeDao.selectCommentsByIssues(sqlSession, issuesByKey.keySet()); + for (DefaultIssueComment comment : comments) { + DefaultIssue issue = issuesByKey.get(comment.issueKey()); + issue.addComment(comment); + } + return new DefaultResults(issues, findRules(ruleIds), findComponents(componentIds), diff --git a/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesInternal.java b/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesInternal.java index a6590440c30..2a00bcf6213 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesInternal.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/WebIssuesInternal.java @@ -68,16 +68,16 @@ public class WebIssuesInternal implements ServerComponent { return actions.plan(issueKey, actionPlanKey, UserSession.get()); } - public IssueComment addComment(String issueKey, String comment) { - return actions.addComment(issueKey, comment, UserSession.get()); + public IssueComment addComment(String issueKey, String text) { + return actions.addComment(issueKey, text, UserSession.get()); } - public DefaultIssueComment[] comments(String issueKey) { - return actions.comments(issueKey, UserSession.get()); + public IssueComment deleteComment(String commentKey) { + return actions.deleteComment(commentKey, UserSession.get()); } - public FieldDiffs[] changes(String issueKey) { - return actions.changes(issueKey, UserSession.get()); + public IssueComment editComment(String commentKey, String newText) { + return actions.editComment(commentKey, newText, UserSession.get()); } public Issue create(Map parameters) { @@ -121,9 +121,9 @@ public class WebIssuesInternal implements ServerComponent { Integer projectId = toInteger(parameters.get("projectId")); DefaultActionPlan actionPlan = DefaultActionPlan.create(parameters.get("name")) - .setDescription(parameters.get("description")) - .setUserLogin(UserSession.get().login()) - .setDeadLine(toDate(parameters.get("deadLine"))); + .setDescription(parameters.get("description")) + .setUserLogin(UserSession.get().login()) + .setDeadLine(toDate(parameters.get("deadLine"))); return actionPlanManager.create(actionPlan, projectId); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb index b8568b5a37f..c08e59ad5d6 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb @@ -28,15 +28,14 @@ class Api::IssuesController < Api::ApiController # def search results = Api.issues.find(params) - render :json => jsonp( - { - :securityExclusions => results.securityExclusions, - :paging => paging_to_json(results.paging), - :issues => results.issues.map { |issue| issue_to_json(issue) }, - :rules => results.rules.map { |rule| rule_to_json(rule) }, - :actionPlans => results.actionPlans.map { |action_plan| action_plan_to_json(action_plan) } - } - ) + json = { + :securityExclusions => results.securityExclusions, + :paging => paging_to_json(results.paging), + :issues => results.issues.map { |issue| issue_to_json(issue) }, + :rules => results.rules.map { |rule| rule_to_json(rule) } + } + json[:actionPlans] = results.actionPlans.map { |plan| action_plan_to_json(plan) } if results.actionPlans.size>0 + render :json => jsonp(json) end # @@ -51,9 +50,9 @@ class Api::IssuesController < Api::ApiController issue_key = params[:issue] transitions = Internal.issues.listTransitions(issue_key) render :json => jsonp( - { - :transitions => transitions.map { |t| t.key() } - } + { + :transitions => transitions.map { |t| t.key() } + } ) end @@ -70,7 +69,7 @@ class Api::IssuesController < Api::ApiController issue = Internal.issues.doTransition(params[:issue], params[:transition]) if issue render :json => jsonp({ - :issue => issue_to_json(issue) + :issue => issue_to_json(issue) }) else render :status => 400 @@ -82,7 +81,7 @@ class Api::IssuesController < Api::ApiController # # -- Mandatory parameters # 'issue' is the key of an existing issue - # 'text' is the plain-text message. It can also be set in the post body. + # 'text' is the markdown message. It can be set as an URL parameter or as the post request body. # # -- Example # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/add_comment?issue=4a2881e7-825e-4140-a154-01f420c43d11&text=foooo' @@ -97,20 +96,39 @@ class Api::IssuesController < Api::ApiController end # - # GET /api/issues/comments?issue= + # POST /api/issues/delete_comment?key= + # + # -- Mandatory parameters + # 'key' is the comment key # # -- Example - # curl -v -u admin:admin 'http://localhost:9000/api/issues/comments?issue=4a2881e7-825e-4140-a154-01f420c43d11' + # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/delete_comment?key=392160d3-a4f2-4c52-a565-e4542cfa2096' # - def comments - require_parameters :issue + def delete_comment + verify_post_request + require_parameters :key - comments = Internal.issues.comments(params[:issue]) - render :json => jsonp( - { - :comments => comments.map { |comment| comment_to_json(comment) } - } - ) + comment = Internal.issues.deleteComment(params[:key]) + render :json => jsonp({:comment => comment_to_json(comment)}) + end + + # + # POST /api/issues/edit_comment?key=&text= + # + # -- Mandatory parameters + # 'key' is the comment key + # 'text' is the new value + # + # -- Example + # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/edit_comment?key=392160d3-a4f2-4c52-a565-e4542cfa2096&text=foo' + # + def edit_comment + verify_post_request + require_parameters :key, :text + + text = Api::Utils.read_post_request_param(params[:text]) + comment = Internal.issues.editComment(params[:issue], text) + render :json => jsonp({:comment => comment_to_json(comment)}) end # @@ -198,10 +216,10 @@ class Api::IssuesController < Api::ApiController def issue_to_json(issue) json = { - :key => issue.key, - :component => issue.componentKey, - :rule => issue.ruleKey.toString(), - :status => issue.status + :key => issue.key, + :component => issue.componentKey, + :rule => issue.ruleKey.toString(), + :status => issue.status } json[:actionPlan] = issue.actionPlanKey if issue.actionPlanKey json[:resolution] = issue.resolution if issue.resolution @@ -216,18 +234,29 @@ class Api::IssuesController < Api::ApiController json[:closeDate] = format_java_datetime(issue.closeDate) if issue.closeDate json[:attr] = issue.attributes.to_hash unless issue.attributes.isEmpty() json[:manual] = issue.manual if issue.manual + if issue.comments.size>0 + json[:comments] = issue.comments.map { |c| comment_to_json(c) } + end json end def comment_to_json(comment) { - :key => comment.key(), - :text => comment.text(), - :createdAt => format_java_datetime(comment.createdAt()), - :login => comment.userLogin() + :key => comment.key(), + :login => comment.userLogin(), + :htmlText => comment.text(), + :createdAt => format_java_datetime(comment.createdAt()) } end + def diffs_to_json(diffs) + json = { + :login => diffs.userLogin(), + :at => format_java_datetime(diffs.createdAt()) + } + json + end + def rule_to_json(rule) l10n_name = Internal.rules.ruleL10nName(rule) l10n_desc = Internal.rules.ruleL10nDescription(rule) @@ -249,10 +278,10 @@ class Api::IssuesController < Api::ApiController def paging_to_json(paging) { - :pageIndex => paging.pageIndex, - :pageSize => paging.pageSize, - :total => paging.total, - :pages => paging.pages + :pageIndex => paging.pageIndex, + :pageSize => paging.pageSize, + :total => paging.total, + :pages => paging.pages } end diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java index 2654bf42fcd..6815241cbf5 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueFinderTest.java @@ -33,6 +33,7 @@ import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; import org.sonar.core.issue.ActionPlanManager; import org.sonar.core.issue.DefaultActionPlan; +import org.sonar.core.issue.db.IssueChangeDao; import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; import org.sonar.core.persistence.MyBatis; @@ -58,11 +59,12 @@ public class ServerIssueFinderTest { MyBatis mybatis = mock(MyBatis.class); IssueDao issueDao = mock(IssueDao.class); + IssueChangeDao issueChangeDao = mock(IssueChangeDao.class); AuthorizationDao authorizationDao = mock(AuthorizationDao.class); DefaultRuleFinder ruleFinder = mock(DefaultRuleFinder.class); ResourceDao resourceDao = mock(ResourceDao.class); ActionPlanManager actionPlanManager = mock(ActionPlanManager.class); - ServerIssueFinder finder = new ServerIssueFinder(mybatis, issueDao, authorizationDao, ruleFinder, resourceDao, actionPlanManager); + ServerIssueFinder finder = new ServerIssueFinder(mybatis, issueDao, issueChangeDao, authorizationDao, ruleFinder, resourceDao, actionPlanManager); @Test public void should_find_issues() { @@ -78,7 +80,7 @@ public class ServerIssueFinderTest { .setRuleKey_unit_test_only("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(dtoList); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); IssueFinder.Results results = finder.find(query, null, UserRole.USER); @@ -102,7 +104,7 @@ public class ServerIssueFinderTest { .setRuleKey_unit_test_only("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(dtoList); when(authorizationDao.keepAuthorizedComponentIds(anySet(), anyInt(), anyString(), any(SqlSession.class))).thenReturn(newHashSet(123)); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(newArrayList(issue1)); @@ -127,7 +129,7 @@ public class ServerIssueFinderTest { .setRuleKey_unit_test_only("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(dtoList); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); IssueFinder.Results results = finder.find(query, null, UserRole.USER); @@ -170,7 +172,7 @@ public class ServerIssueFinderTest { .setRuleKey_unit_test_only("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(dtoList); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); IssueFinder.Results results = finder.find(query, null, UserRole.USER); @@ -198,7 +200,7 @@ public class ServerIssueFinderTest { .setRuleKey_unit_test_only("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(dtoList); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); IssueFinder.Results results = finder.find(query, null, UserRole.USER); @@ -226,7 +228,7 @@ public class ServerIssueFinderTest { .setRuleKey_unit_test_only("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(dtoList); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(dtoList); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); when(actionPlanManager.findByKeys(anyCollection())).thenReturn(newArrayList(actionPlan1, actionPlan2)); @@ -241,7 +243,7 @@ public class ServerIssueFinderTest { public void should_get_empty_result_when_no_issue() { grantAccessRights(); IssueQuery query = IssueQuery.builder().build(); - when(issueDao.selectIssueIdsAndComponentsId(eq(query), any(SqlSession.class))).thenReturn(Collections.emptyList()); + when(issueDao.selectIssueAndComponentIds(eq(query), any(SqlSession.class))).thenReturn(Collections.emptyList()); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(Collections.emptyList()); diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java index cf82f66e74e..68c9a926e4f 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java @@ -20,11 +20,11 @@ package org.sonar.wsclient.issue; import com.github.kevinsawicki.http.HttpRequest; +import org.json.simple.JSONValue; import org.sonar.wsclient.internal.EncodingUtils; import org.sonar.wsclient.internal.HttpRequestFactory; import javax.annotation.Nullable; - import java.util.List; import java.util.Map; @@ -88,6 +88,17 @@ public class DefaultIssueClient implements IssueClient { } } + @Override + public IssueComment addComment(String issueKey, String markdownText) { + Map params = EncodingUtils.toMap("issue", issueKey, "text", markdownText); + HttpRequest request = requestFactory.post("/api/issues/add_comment", params); + if (!request.ok()) { + throw new IllegalStateException("Fail to add issue comment. Bad HTTP response status: " + request.code()); + } + String json = request.body(); + return new IssueComment((Map) JSONValue.parse(json)); + } + @Override public List transitions(String issueKey) { Map queryParams = EncodingUtils.toMap("issue", issueKey); diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issue.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issue.java index d22ed22afbb..ad922698f50 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issue.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issue.java @@ -22,9 +22,7 @@ package org.sonar.wsclient.issue; import org.sonar.wsclient.unmarshallers.JsonUtils; import javax.annotation.CheckForNull; -import java.util.Collections; -import java.util.Date; -import java.util.Map; +import java.util.*; /** * @since 3.6 @@ -66,11 +64,6 @@ public class Issue { return JsonUtils.getInteger(json, "line"); } - // TODO to be removed - public Double cost() { - return JsonUtils.getDouble(json, "effortToFix"); - } - @CheckForNull public Double effortToFix() { return JsonUtils.getDouble(json, "effortToFix"); @@ -129,4 +122,18 @@ public class Issue { } return attr; } + + /** + * Non-null list of comments + */ + public List comments() { + List comments = new ArrayList(); + List jsonComments = (List) json.get("comments"); + if (jsonComments != null) { + for (Map jsonComment : jsonComments) { + comments.add(new IssueComment(jsonComment)); + } + } + return comments; + } } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java index 805ba49fbef..324c9e79132 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java @@ -36,6 +36,8 @@ public interface IssueClient { void plan(String issueKey, @Nullable String actionPlan); + IssueComment addComment(String issueKey, String markdownText); + void create(NewIssue issue); List transitions(String issueKey); diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueComment.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueComment.java new file mode 100644 index 00000000000..f649d573b8f --- /dev/null +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueComment.java @@ -0,0 +1,52 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.wsclient.issue; + +import org.sonar.wsclient.unmarshallers.JsonUtils; + +import java.util.Date; +import java.util.Map; + +/** + * @since 3.6 + */ +public class IssueComment { + private final Map json; + + IssueComment(Map json) { + this.json = json; + } + + public String key() { + return JsonUtils.getString(json, "key"); + } + + public String htmlText() { + return JsonUtils.getString(json, "htmlText"); + } + + public String login() { + return JsonUtils.getString(json, "login"); + } + + public Date createdAt() { + return JsonUtils.getDateTime(json, "createdAt"); + } +} diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueParser.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueParser.java index 4fa2517c528..5f0a9a2e16a 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueParser.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueParser.java @@ -27,6 +27,9 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +/** + * @since 3.6 + */ class IssueParser { Issues parseIssues(String json) { diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issues.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issues.java index b5db438fa63..8282d39b723 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issues.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/Issues.java @@ -42,6 +42,10 @@ public class Issues { return list; } + public int size() { + return list.size(); + } + Issues add(Rule rule) { rulesByKey.put(rule.key(), rule); return this; diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueParserTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueParserTest.java index 276266f5c1d..c7e05d26332 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueParserTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueParserTest.java @@ -20,8 +20,10 @@ package org.sonar.wsclient.issue; import org.apache.commons.io.IOUtils; +import org.apache.http.impl.cookie.DateUtils; import org.junit.Test; +import java.util.Date; import java.util.List; import static org.fest.assertions.Assertions.assertThat; @@ -52,6 +54,7 @@ public class IssueParserTest { assertThat(first.attribute("JIRA")).isEqualTo("FOO-1234"); assertThat(first.attribute("OTHER")).isNull(); assertThat(first.attributes()).hasSize(1); + assertThat(first.comments()).isEmpty(); Issue second = list.get(1); assertThat(second.key()).isEqualTo("FGHIJ"); @@ -60,6 +63,7 @@ public class IssueParserTest { assertThat(second.description()).isNull(); assertThat(second.attribute("JIRA")).isNull(); assertThat(second.attributes()).isEmpty(); + assertThat(second.comments()).isEmpty(); assertThat(issues.rules()).hasSize(2); assertThat(issues.rule(first).key()).isEqualTo("squid:CycleBetweenPackages"); @@ -96,4 +100,25 @@ public class IssueParserTest { assertThat(transitions).containsOnly("resolve", "falsepositive"); } + @Test + public void should_parse_comments() throws Exception { + String json = IOUtils.toString(getClass().getResourceAsStream("/org/sonar/wsclient/issue/IssueParserTest/issue-with-comments.json")); + Issues issues = new IssueParser().parseIssues(json); + assertThat(issues.size()).isEqualTo(1); + + Issue issue = issues.list().get(0); + assertThat(issue.comments()).hasSize(2); + + IssueComment firstComment = issue.comments().get(0); + assertThat(firstComment.key()).isEqualTo("COMMENT-1"); + assertThat(firstComment.login()).isEqualTo("morgan"); + assertThat(firstComment.htmlText()).isEqualTo("the first comment"); + assertThat(firstComment.createdAt().getDate()).isEqualTo(18); + + IssueComment secondComment = issue.comments().get(1); + assertThat(secondComment.key()).isEqualTo("COMMENT-2"); + assertThat(secondComment.login()).isEqualTo("arthur"); + assertThat(secondComment.htmlText()).isEqualTo("the second comment"); + assertThat(secondComment.createdAt().getDate()).isEqualTo(19); + } } diff --git a/sonar-ws-client/src/test/resources/org/sonar/wsclient/issue/IssueParserTest/issue-with-comments.json b/sonar-ws-client/src/test/resources/org/sonar/wsclient/issue/IssueParserTest/issue-with-comments.json new file mode 100644 index 00000000000..914e7627e7c --- /dev/null +++ b/sonar-ws-client/src/test/resources/org/sonar/wsclient/issue/IssueParserTest/issue-with-comments.json @@ -0,0 +1,41 @@ +{ + "issues": [ + { + "key": "ABCDE", + "component": "Action.java", + "rule": "squid:CycleBetweenPackages", + "severity": "CRITICAL", + "status": "OPEN", + "comments": [ + { + "key": "COMMENT-1", + "login": "morgan", + "htmlText": "the first comment", + "createdAt": "2013-05-18T13:45:34+0200" + }, + { + "key": "COMMENT-2", + "login": "arthur", + "htmlText": "the second comment", + "createdAt": "2013-06-19T00:02:03+0100" + } + ] + } + ], + "rules": [ + { + + "key": "squid:CycleBetweenPackages", + "name": "Avoid cycle between java packages", + "desc": "

\nWhen several packages are involved in a cycle (package A > package B > package C > package A where \">\" means \"depends upon\"),\nthat means that those packages are highly coupled and that there is no way to reuse/extract one of those packages without importing all the other packages.\nSuch cycle could quickly increase the effort required to maintain an application and to embrace business change.\nSonar not only detect cycles between packages but also determines what is the minimum effort to break those cycles.\nThis rule log a violation on each source file having an outgoing dependency to be but in order to break a cycle.\n

\n" + + } + ], + "paging": { + "pageIndex": 1, + "pageSize": 100, + "total": 2, + "pages": 1 + }, + "securityExclusions": true +} \ No newline at end of file -- 2.39.5