diff options
author | Simon Brandhof <simon.brandhof@sonarsource.com> | 2015-06-30 09:02:14 +0200 |
---|---|---|
committer | Simon Brandhof <simon.brandhof@sonarsource.com> | 2015-07-02 19:44:28 +0200 |
commit | 1c0b9a97e3bc6d58c4697b4350b39adc81a6efc1 (patch) | |
tree | 58dc9e0251604655638d5f9f47611116e6e49cd1 /sonar-core | |
parent | 44a34aca80bb01c3a7689fe02d34f17718bf9293 (diff) | |
download | sonarqube-1c0b9a97e3bc6d58c4697b4350b39adc81a6efc1.tar.gz sonarqube-1c0b9a97e3bc6d58c4697b4350b39adc81a6efc1.zip |
SONAR-6588 move computation of debt to Compute Engine
Diffstat (limited to 'sonar-core')
20 files changed, 638 insertions, 106 deletions
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 3e7c21e8d23..23018566dc4 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 @@ -65,11 +65,11 @@ public class IssueChangeDao implements DaoComponent { } } - public List<IssueChangeDto> selectChangelogOfUnresolvedIssuesByComponent(String componentUuid) { + public List<IssueChangeDto> selectChangelogOfNonClosedIssuesByComponent(String componentUuid) { DbSession session = mybatis.openSession(false); try { IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); - return mapper.selectChangelogOfUnresolvedIssuesByComponent(componentUuid, IssueChangeDto.TYPE_FIELD_CHANGE); + return mapper.selectChangelogOfNonClosedIssuesByComponent(componentUuid, IssueChangeDto.TYPE_FIELD_CHANGE); } 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 732ddc5c75b..61d6b8e01ab 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 @@ -41,5 +41,5 @@ public interface IssueChangeMapper { List<IssueChangeDto> selectByIssuesAndType(@Param("issueKeys") List<String> issueKeys, @Param("changeType") String changeType); - List<IssueChangeDto> selectChangelogOfUnresolvedIssuesByComponent(@Param("componentUuid") String componentUuid, @Param("changeType") String changeType); + List<IssueChangeDto> selectChangelogOfNonClosedIssuesByComponent(@Param("componentUuid") String componentUuid, @Param("changeType") String changeType); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java b/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java index 7c6b48a1603..ccc84139154 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java @@ -35,7 +35,7 @@ public class UpdateConflictResolver { private static final Logger LOG = Loggers.get(UpdateConflictResolver.class); public void resolve(DefaultIssue issue, IssueMapper mapper) { - LOG.debug("Resolve conflict on issue " + issue.key()); + LOG.debug("Resolve conflict on issue {}", issue.key()); IssueDto dbIssue = mapper.selectByKey(issue.key()); if (dbIssue != null) { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockHashSequence.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockHashSequence.java index 7a4d38671ca..9a9a179378f 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockHashSequence.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockHashSequence.java @@ -20,6 +20,7 @@ package org.sonar.core.issue.tracking; import java.util.List; +import javax.annotation.Nullable; public class BlockHashSequence { @@ -57,6 +58,10 @@ public class BlockHashSequence { return blockHashes[line - 1]; } + public boolean hasLine(@Nullable Integer line) { + return (line != null) && (line > 0) && (line <= blockHashes.length); + } + private static class BlockHashFactory { private static final int PRIME_BASE = 31; diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java index d4361819fbb..55138a34995 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java @@ -36,22 +36,22 @@ class BlockRecognizer<RAW extends Trackable, BASE extends Trackable> { * Only the issues associated to a line can be matched here. */ void match(Input<RAW> rawInput, Input<BASE> baseInput, Tracking<RAW, BASE> tracking) { - BlockHashSequence baseHashSequence = baseInput.getBlockHashSequence(); BlockHashSequence rawHashSequence = rawInput.getBlockHashSequence(); + BlockHashSequence baseHashSequence = baseInput.getBlockHashSequence(); - Multimap<Integer, RAW> rawsByLine = groupByLine(tracking.getUnmatchedRaws()); - Multimap<Integer, BASE> basesByLine = groupByLine(tracking.getUnmatchedBases()); - Map<Integer, HashOccurrence> map = new HashMap<>(); + Multimap<Integer, RAW> rawsByLine = groupByLine(tracking.getUnmatchedRaws(), rawHashSequence); + Multimap<Integer, BASE> basesByLine = groupByLine(tracking.getUnmatchedBases(), baseHashSequence); + Map<Integer, HashOccurrence> occurrencesByHash = new HashMap<>(); for (Integer line : basesByLine.keySet()) { int hash = baseHashSequence.getBlockHashForLine(line); - HashOccurrence hashOccurrence = map.get(hash); + HashOccurrence hashOccurrence = occurrencesByHash.get(hash); if (hashOccurrence == null) { // first occurrence in base hashOccurrence = new HashOccurrence(); hashOccurrence.baseLine = line; hashOccurrence.baseCount = 1; - map.put(hash, hashOccurrence); + occurrencesByHash.put(hash, hashOccurrence); } else { hashOccurrence.baseCount++; } @@ -59,14 +59,14 @@ class BlockRecognizer<RAW extends Trackable, BASE extends Trackable> { for (Integer line : rawsByLine.keySet()) { int hash = rawHashSequence.getBlockHashForLine(line); - HashOccurrence hashOccurrence = map.get(hash); + HashOccurrence hashOccurrence = occurrencesByHash.get(hash); if (hashOccurrence != null) { hashOccurrence.rawLine = line; hashOccurrence.rawCount++; } } - for (HashOccurrence hashOccurrence : map.values()) { + for (HashOccurrence hashOccurrence : occurrencesByHash.values()) { if (hashOccurrence.baseCount == 1 && hashOccurrence.rawCount == 1) { // Guaranteed that baseLine has been moved to rawLine, so we can map all issues on baseLine to all issues on rawLine map(rawsByLine.get(hashOccurrence.rawLine), basesByLine.get(hashOccurrence.baseLine), tracking); @@ -131,11 +131,11 @@ class BlockRecognizer<RAW extends Trackable, BASE extends Trackable> { } } - private static <T extends Trackable> Multimap<Integer, T> groupByLine(Collection<T> trackables) { + private static <T extends Trackable> Multimap<Integer, T> groupByLine(Collection<T> trackables, BlockHashSequence hashSequence) { Multimap<Integer, T> result = LinkedHashMultimap.create(); for (T trackable : trackables) { Integer line = trackable.getLine(); - if (line != null) { + if (hashSequence.hasLine(line)) { result.put(line, trackable); } } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java index 5da2ca5da04..e7cd196bbca 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java @@ -25,11 +25,11 @@ import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Multimap; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.Objects; import javax.annotation.Nonnull; import org.apache.commons.lang.StringUtils; import org.sonar.api.rule.RuleKey; -import org.sonar.api.utils.log.Loggers; import static com.google.common.collect.FluentIterable.from; @@ -38,6 +38,8 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { public Tracking<RAW, BASE> track(Input<RAW> rawInput, Input<BASE> baseInput) { Tracking<RAW, BASE> tracking = new Tracking<>(rawInput, baseInput); + relocateManualIssues(rawInput, baseInput, tracking); + // 1. match issues with same rule, same line and same line hash, but not necessarily with same message match(tracking, LineAndLineHashKeyFactory.INSTANCE); @@ -54,9 +56,6 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { // See SONAR-2812 match(tracking, LineHashKeyFactory.INSTANCE); - // TODO what about issues on line 0 ? - relocateManualIssues(rawInput, tracking); - return tracking; } @@ -92,23 +91,28 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { tracking.markRawsAsAssociated(trackedRaws); } - private void relocateManualIssues(Input<RAW> rawInput, Tracking<RAW, BASE> tracking) { - Iterable<BASE> manualIssues = from(tracking.getUnmatchedBases()).filter(IsManual.INSTANCE); + private void relocateManualIssues(Input<RAW> rawInput, Input<BASE> baseInput, Tracking<RAW, BASE> tracking) { + // FIXME copy of Set if required to avoid concurrent modifications (see tracking.associateManualIssueToLine()) + Iterable<BASE> manualIssues = from(new HashSet<>(tracking.getUnmatchedBases())).filter(IsManual.INSTANCE); for (BASE base : manualIssues) { if (base.getLine() == null) { // no need to relocate. Location is unchanged. - tracking.associateManualIssueToLine(base, 0); + tracking.associateManualIssueToLine(base, null); } else { - String lineHash = base.getLineHash(); - if (!Strings.isNullOrEmpty(lineHash)) { - int[] rawLines = rawInput.getLineHashSequence().getLinesForHash(lineHash); + String baseHash = base.getLineHash(); + if (Strings.isNullOrEmpty(baseHash)) { + baseHash = baseInput.getLineHashSequence().getHashForLine(base.getLine()); + } + if (!Strings.isNullOrEmpty(baseHash)) { + int[] rawLines = rawInput.getLineHashSequence().getLinesForHash(baseHash); if (rawLines.length == 1) { tracking.associateManualIssueToLine(base, rawLines[0]); - } else if (rawLines.length == 0 && base.getLine() <= rawInput.getLineHashSequence().length()) { + } else if (rawLines.length == 0 && rawInput.getLineHashSequence().hasLine(base.getLine())) { // still valid (???). We didn't manage to correctly detect code move, so the // issue is kept at the same location, even if code changes tracking.associateManualIssueToLine(base, base.getLine()); } + // TODO if hash found multiple times, , pick the closest line } } } @@ -147,13 +151,9 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { } LineAndLineHashKey that = (LineAndLineHashKey) o; // start with most discriminant field - if (!Objects.equals(line, that.line)) { - return false; - } - if (!lineHash.equals(that.lineHash)) { - return false; - } - return ruleKey.equals(that.ruleKey); + return Objects.equals(line, that.line) + && lineHash.equals(that.lineHash) + && ruleKey.equals(that.ruleKey); } @Override @@ -175,7 +175,8 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { private static class LineHashAndMessageKey implements SearchKey { private final RuleKey ruleKey; - private final String message, lineHash; + private final String message; + private final String lineHash; LineHashAndMessageKey(Trackable trackable) { this.ruleKey = trackable.getRuleKey(); @@ -190,13 +191,9 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { } LineHashAndMessageKey that = (LineHashAndMessageKey) o; // start with most discriminant field - if (!lineHash.equals(that.lineHash)) { - return false; - } - if (!message.equals(that.message)) { - return false; - } - return ruleKey.equals(that.ruleKey); + return lineHash.equals(that.lineHash) + && message.equals(that.message) + && ruleKey.equals(that.ruleKey); } @Override @@ -234,13 +231,9 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { } LineAndMessageKey that = (LineAndMessageKey) o; // start with most discriminant field - if (!Objects.equals(line, that.line)) { - return false; - } - if (!message.equals(that.message)) { - return false; - } - return ruleKey.equals(that.ruleKey); + return Objects.equals(line, that.line) + && message.equals(that.message) + && ruleKey.equals(that.ruleKey); } @Override @@ -276,10 +269,8 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> { } LineAndLineHashKey that = (LineAndLineHashKey) o; // start with most discriminant field - if (!lineHash.equals(that.lineHash)) { - return false; - } - return ruleKey.equals(that.ruleKey); + return lineHash.equals(that.lineHash) + && ruleKey.equals(that.ruleKey); } @Override diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java index f2a5306e500..579c6f51623 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java @@ -28,6 +28,7 @@ import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; public class Tracking<RAW extends Trackable, BASE extends Trackable> { @@ -112,7 +113,7 @@ public class Tracking<RAW extends Trackable, BASE extends Trackable> { return openManualIssues; } - void associateManualIssueToLine(BASE manualIssue, int line) { + void associateManualIssueToLine(BASE manualIssue, @Nullable Integer line) { openManualIssues.put(line, manualIssue); unmatchedBases.remove(manualIssue); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java index 5b54986c84f..5d5275f8502 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java @@ -157,8 +157,8 @@ public class IssueWorkflow implements Startable { .build()) .transition(Transition.builder("automaticclosemanual") .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_CLOSED) - .conditions(new NotCondition(IsBeingClosed.INSTANCE), IsManual.INSTANCE) - .functions(new SetCloseDate(true)) + .conditions(IsManual.INSTANCE) + .functions(SetClosed.INSTANCE, new SetCloseDate(true)) .automatic() .build()) diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetClosed.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetClosed.java index 49b874220e7..06b71d28ff0 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetClosed.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetClosed.java @@ -28,9 +28,6 @@ public enum SetClosed implements Function { @Override public void execute(Context context) { DefaultIssue issue = (DefaultIssue) context.issue(); - if (!issue.isBeingClosed()) { - throw new IllegalStateException("Issue is still open: " + issue); - } if (issue.isOnDisabledRule()) { context.setResolution(Issue.RESOLUTION_REMOVED); } else { diff --git a/sonar-core/src/main/java/org/sonar/core/measure/db/MeasureDto.java b/sonar-core/src/main/java/org/sonar/core/measure/db/MeasureDto.java index 04b36bf3ef3..1e288b3cea3 100644 --- a/sonar-core/src/main/java/org/sonar/core/measure/db/MeasureDto.java +++ b/sonar-core/src/main/java/org/sonar/core/measure/db/MeasureDto.java @@ -20,6 +20,7 @@ package org.sonar.core.measure.db; +import com.google.common.base.Objects; import java.nio.charset.StandardCharsets; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -239,4 +240,29 @@ public class MeasureDto { return this; } + @Override + public String toString() { + return Objects.toStringHelper(this) + .add("id", id) + .add("value", value) + .add("textValue", textValue) + .add("dataValue", dataValue) + .add("variation1", variation1) + .add("variation2", variation2) + .add("variation3", variation3) + .add("variation4", variation4) + .add("variation5", variation5) + .add("alertStatus", alertStatus) + .add("alertText", alertText) + .add("description", description) + .add("componentId", componentId) + .add("snapshotId", snapshotId) + .add("metricId", metricId) + .add("ruleId", ruleId) + .add("characteristicId", characteristicId) + .add("personId", personId) + .add("metricKey", metricKey) + .add("componentKey", componentKey) + .toString(); + } } diff --git a/sonar-core/src/main/java/org/sonar/core/computation/package-info.java b/sonar-core/src/main/java/org/sonar/core/rule/RuleKeyFunctions.java index 218db23b3fb..9a4f9c0ea04 100644 --- a/sonar-core/src/main/java/org/sonar/core/computation/package-info.java +++ b/sonar-core/src/main/java/org/sonar/core/rule/RuleKeyFunctions.java @@ -17,8 +17,33 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +package org.sonar.core.rule; -@ParametersAreNonnullByDefault -package org.sonar.core.computation; +import com.google.common.base.Function; +import javax.annotation.Nonnull; +import org.sonar.api.rule.RuleKey; -import javax.annotation.ParametersAreNonnullByDefault; +public final class RuleKeyFunctions { + + private RuleKeyFunctions() { + // only static methods + } + + public static Function<String, RuleKey> stringToRuleKey() { + return StringToRuleKey.INSTANCE; + } + + /** + * Transforms a string representation of key to {@link RuleKey}. It + * does not accept null string inputs. + */ + private enum StringToRuleKey implements Function<String, RuleKey> { + INSTANCE; + @Nonnull + @Override + public RuleKey apply(@Nonnull String input) { + return RuleKey.parse(input); + } + } + +} 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 e810f1dc33c..b4aa445dae3 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 @@ -58,14 +58,14 @@ order by created_at asc </select> - <select id="selectChangelogOfUnresolvedIssuesByComponent" parameterType="map" resultType="IssueChange"> + <select id="selectChangelogOfNonClosedIssuesByComponent" parameterType="map" resultType="IssueChange"> select <include refid="issueChangeColumns"/> from issue_changes c inner join issues i on i.kee = c.issue_key where i.component_uuid=#{componentUuid} and c.change_type=#{changeType} - and i.resolution is null + and i.status <> 'CLOSED' </select> </mapper> diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java new file mode 100644 index 00000000000..4572e8ac599 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java @@ -0,0 +1,219 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.core.issue; + +import com.google.common.collect.ImmutableMap; +import org.apache.commons.lang.StringUtils; +import org.junit.Test; +import org.sonar.api.issue.Issue; +import org.sonar.api.issue.IssueComment; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.utils.Duration; + +import java.text.SimpleDateFormat; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; + +public class DefaultIssueTest { + + DefaultIssue issue = new DefaultIssue(); + + @Test + public void test_setters_and_getters() throws Exception { + issue.setKey("ABCD") + .setComponentKey("org.sample.Sample") + .setProjectKey("Sample") + .setRuleKey(RuleKey.of("squid", "S100")) + .setLanguage("xoo") + .setSeverity("MINOR") + .setManualSeverity(true) + .setMessage("a message") + .setLine(7) + .setEffortToFix(1.2d) + .setDebt(Duration.create(28800L)) + .setActionPlanKey("BCDE") + .setStatus(Issue.STATUS_CLOSED) + .setResolution(Issue.RESOLUTION_FIXED) + .setReporter("simon") + .setAssignee("julien") + .setAuthorLogin("steph") + .setChecksum("c7b5db46591806455cf082bb348631e8") + .setNew(true) + .setBeingClosed(true) + .setOnDisabledRule(true) + .setChanged(true) + .setSendNotifications(true) + .setCreationDate(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-19")) + .setUpdateDate(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-20")) + .setCloseDate(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-21")) + .setSelectedAt(1400000000000L); + + assertThat(issue.key()).isEqualTo("ABCD"); + assertThat(issue.componentKey()).isEqualTo("org.sample.Sample"); + assertThat(issue.projectKey()).isEqualTo("Sample"); + assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("squid", "S100")); + assertThat(issue.language()).isEqualTo("xoo"); + assertThat(issue.severity()).isEqualTo("MINOR"); + assertThat(issue.manualSeverity()).isTrue(); + assertThat(issue.message()).isEqualTo("a message"); + assertThat(issue.line()).isEqualTo(7); + assertThat(issue.effortToFix()).isEqualTo(1.2d); + assertThat(issue.debt()).isEqualTo(Duration.create(28800L)); + assertThat(issue.actionPlanKey()).isEqualTo("BCDE"); + assertThat(issue.status()).isEqualTo(Issue.STATUS_CLOSED); + assertThat(issue.resolution()).isEqualTo(Issue.RESOLUTION_FIXED); + assertThat(issue.reporter()).isEqualTo("simon"); + assertThat(issue.assignee()).isEqualTo("julien"); + assertThat(issue.authorLogin()).isEqualTo("steph"); + assertThat(issue.checksum()).isEqualTo("c7b5db46591806455cf082bb348631e8"); + assertThat(issue.isNew()).isTrue(); + assertThat(issue.isBeingClosed()).isTrue(); + assertThat(issue.isOnDisabledRule()).isTrue(); + assertThat(issue.isChanged()).isTrue(); + assertThat(issue.mustSendNotifications()).isTrue(); + assertThat(issue.creationDate()).isEqualTo(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-19")); + assertThat(issue.updateDate()).isEqualTo(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-20")); + assertThat(issue.closeDate()).isEqualTo(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-21")); + assertThat(issue.selectedAt()).isEqualTo(1400000000000L); + } + + @Test + public void set_empty_dates() { + issue + .setCreationDate(null) + .setUpdateDate(null) + .setCloseDate(null) + .setSelectedAt(null); + + assertThat(issue.creationDate()).isNull(); + assertThat(issue.updateDate()).isNull(); + assertThat(issue.closeDate()).isNull(); + assertThat(issue.selectedAt()).isNull(); + } + + @Test + public void test_attributes() throws Exception { + assertThat(issue.attribute("foo")).isNull(); + issue.setAttribute("foo", "bar"); + assertThat(issue.attribute("foo")).isEqualTo("bar"); + issue.setAttribute("foo", "newbar"); + assertThat(issue.attribute("foo")).isEqualTo("newbar"); + issue.setAttribute("foo", null); + assertThat(issue.attribute("foo")).isNull(); + } + + @Test + public void setAttributes_should_not_clear_existing_values() { + issue.setAttributes(ImmutableMap.of("1", "one")); + assertThat(issue.attribute("1")).isEqualTo("one"); + + issue.setAttributes(ImmutableMap.of("2", "two")); + assertThat(issue.attributes()).containsOnly(entry("1", "one"), entry("2", "two")); + + issue.setAttributes(null); + assertThat(issue.attributes()).containsOnly(entry("1", "one"), entry("2", "two")); + } + + @Test + public void fail_on_empty_status() { + try { + issue.setStatus(""); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Status must be set"); + } + } + + @Test + public void fail_on_bad_severity() { + try { + issue.setSeverity("FOO"); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Not a valid severity: FOO"); + } + } + + @Test + public void message_should_be_abbreviated_if_too_long() { + issue.setMessage(StringUtils.repeat("a", 5000)); + assertThat(issue.message()).hasSize(4000); + } + + @Test + public void message_should_be_trimmed() { + issue.setMessage(" foo "); + assertThat(issue.message()).isEqualTo("foo"); + } + + @Test + public void message_could_be_null() { + issue.setMessage(null); + assertThat(issue.message()).isNull(); + } + + @Test + public void test_nullable_fields() throws Exception { + issue.setEffortToFix(null).setSeverity(null).setLine(null); + assertThat(issue.effortToFix()).isNull(); + assertThat(issue.severity()).isNull(); + assertThat(issue.line()).isNull(); + } + + @Test + public void test_equals_and_hashCode() throws Exception { + DefaultIssue a1 = new DefaultIssue().setKey("AAA"); + DefaultIssue a2 = new DefaultIssue().setKey("AAA"); + DefaultIssue b = new DefaultIssue().setKey("BBB"); + assertThat(a1).isEqualTo(a1); + assertThat(a1).isEqualTo(a2); + assertThat(a1).isNotEqualTo(b); + assertThat(a1.hashCode()).isEqualTo(a1.hashCode()); + } + + @Test + public void comments_should_not_be_modifiable() { + DefaultIssue issue = new DefaultIssue().setKey("AAA"); + + List<IssueComment> comments = issue.comments(); + assertThat(comments).isEmpty(); + + try { + comments.add(new DefaultIssueComment()); + fail(); + } catch (UnsupportedOperationException e) { + // ok + } catch (Exception e) { + fail("Unexpected exception: " + e); + } + } + + @Test + public void all_changes_contain_current_change() { + IssueChangeContext issueChangeContext = mock(IssueChangeContext.class); + DefaultIssue issue = new DefaultIssue().setKey("AAA").setFieldChange(issueChangeContext, "actionPlan", "1.0", "1.1"); + + assertThat(issue.changes()).hasSize(1); + } +} diff --git a/sonar-core/src/test/java/org/sonar/core/issue/FieldDiffsTest.java b/sonar-core/src/test/java/org/sonar/core/issue/FieldDiffsTest.java new file mode 100644 index 00000000000..beb207f0385 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/issue/FieldDiffsTest.java @@ -0,0 +1,149 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.core.issue; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class FieldDiffsTest { + + FieldDiffs diffs = new FieldDiffs(); + + @Test + public void diffs_should_be_empty_by_default() { + assertThat(diffs.diffs()).isEmpty(); + } + + @Test + public void test_diff() throws Exception { + diffs.setDiff("severity", "BLOCKER", "INFO"); + diffs.setDiff("resolution", "OPEN", "FIXED"); + + assertThat(diffs.diffs()).hasSize(2); + + FieldDiffs.Diff diff = diffs.diffs().get("severity"); + assertThat(diff.oldValue()).isEqualTo("BLOCKER"); + assertThat(diff.newValue()).isEqualTo("INFO"); + + diff = diffs.diffs().get("resolution"); + assertThat(diff.oldValue()).isEqualTo("OPEN"); + assertThat(diff.newValue()).isEqualTo("FIXED"); + } + + @Test + public void diff_with_long_values() { + diffs.setDiff("technicalDebt", 50l, "100"); + + FieldDiffs.Diff diff = diffs.diffs().get("technicalDebt"); + assertThat(diff.oldValueLong()).isEqualTo(50l); + assertThat(diff.newValueLong()).isEqualTo(100l); + } + + @Test + public void diff_with_empty_long_values() { + diffs.setDiff("technicalDebt", null, ""); + + FieldDiffs.Diff diff = diffs.diffs().get("technicalDebt"); + assertThat(diff.oldValueLong()).isNull(); + assertThat(diff.newValueLong()).isNull(); + } + + @Test + public void test_diff_by_key() throws Exception { + diffs.setDiff("severity", "BLOCKER", "INFO"); + diffs.setDiff("resolution", "OPEN", "FIXED"); + + assertThat(diffs.diffs()).hasSize(2); + + FieldDiffs.Diff diff = diffs.diffs().get("severity"); + assertThat(diff.oldValue()).isEqualTo("BLOCKER"); + assertThat(diff.newValue()).isEqualTo("INFO"); + + diff = diffs.diffs().get("resolution"); + assertThat(diff.oldValue()).isEqualTo("OPEN"); + assertThat(diff.newValue()).isEqualTo("FIXED"); + } + + @Test + public void should_keep_old_value() { + diffs.setDiff("severity", "BLOCKER", "INFO"); + diffs.setDiff("severity", null, "MAJOR"); + FieldDiffs.Diff diff = diffs.diffs().get("severity"); + assertThat(diff.oldValue()).isEqualTo("BLOCKER"); + assertThat(diff.newValue()).isEqualTo("MAJOR"); + } + + @Test + public void test_toString() throws Exception { + diffs.setDiff("severity", "BLOCKER", "INFO"); + diffs.setDiff("resolution", "OPEN", "FIXED"); + + assertThat(diffs.toString()).isEqualTo("severity=BLOCKER|INFO,resolution=OPEN|FIXED"); + } + + @Test + public void test_toString_with_null_values() throws Exception { + diffs.setDiff("severity", null, "INFO"); + diffs.setDiff("assignee", "user1", null); + + assertThat(diffs.toString()).isEqualTo("severity=INFO,assignee="); + } + + @Test + public void test_parse() throws Exception { + diffs = FieldDiffs.parse("severity=BLOCKER|INFO,resolution=OPEN|FIXED"); + assertThat(diffs.diffs()).hasSize(2); + + FieldDiffs.Diff diff = diffs.diffs().get("severity"); + assertThat(diff.oldValue()).isEqualTo("BLOCKER"); + assertThat(diff.newValue()).isEqualTo("INFO"); + + diff = diffs.diffs().get("resolution"); + assertThat(diff.oldValue()).isEqualTo("OPEN"); + assertThat(diff.newValue()).isEqualTo("FIXED"); + } + + @Test + public void test_parse_empty_values() throws Exception { + diffs = FieldDiffs.parse("severity=INFO,resolution="); + assertThat(diffs.diffs()).hasSize(2); + + FieldDiffs.Diff diff = diffs.diffs().get("severity"); + assertThat(diff.oldValue()).isEqualTo(""); + assertThat(diff.newValue()).isEqualTo("INFO"); + + diff = diffs.diffs().get("resolution"); + assertThat(diff.oldValue()).isEqualTo(""); + assertThat(diff.newValue()).isEqualTo(""); + } + + @Test + public void test_parse_null() throws Exception { + diffs = FieldDiffs.parse(null); + assertThat(diffs.diffs()).isEmpty(); + } + + @Test + public void test_parse_empty() throws Exception { + diffs = FieldDiffs.parse(""); + assertThat(diffs.diffs()).isEmpty(); + } +} 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 2d379cfee6c..91530c88983 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 @@ -113,11 +113,11 @@ public class IssueChangeDaoTest extends AbstractDaoTestCase { } @Test - public void selectChangelogOfUnresolvedIssuesByComponent() { - setupData("selectChangelogOfUnresolvedIssuesByComponent"); + public void selectChangelogOfNonClosedIssuesByComponent() { + setupData("selectChangelogOfNonClosedIssuesByComponent"); - List<IssueChangeDto> dtos = dao.selectChangelogOfUnresolvedIssuesByComponent("FILE_1"); - assertThat(dtos).extracting("id").containsExactly(100L); + List<IssueChangeDto> dtos = dao.selectChangelogOfNonClosedIssuesByComponent("FILE_1"); + assertThat(dtos).extracting("id").containsExactly(100L, 103L); } @Test diff --git a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java index a9fe647095e..79a77fbd9e2 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/tracking/TrackerTest.java @@ -19,6 +19,8 @@ */ package org.sonar.core.issue.tracking; +import com.google.common.collect.Iterables; +import com.google.common.collect.Multimap; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -39,6 +41,7 @@ public class TrackerTest { public static final RuleKey RULE_UNUSED_LOCAL_VARIABLE = RuleKey.of("java", "UnusedLocalVariable"); public static final RuleKey RULE_UNUSED_PRIVATE_METHOD = RuleKey.of("java", "UnusedPrivateMethod"); public static final RuleKey RULE_NOT_DESIGNED_FOR_EXTENSION = RuleKey.of("java", "NotDesignedForExtension"); + public static final RuleKey RULE_MANUAL = RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "CodeReview"); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -60,20 +63,6 @@ public class TrackerTest { } @Test - @Ignore - public void different_issues_do_not_match() { - FakeInput baseInput = new FakeInput("H1"); - Issue base = baseInput.createIssueOnLine(1, RULE_SYSTEM_PRINT, "msg1"); - - FakeInput rawInput = new FakeInput("H2", "H3", "H4", "H5", "H6"); - Issue raw = rawInput.createIssueOnLine(5, RULE_SYSTEM_PRINT, "msg2"); - - Tracking<Issue, Issue> tracking = tracker.track(rawInput, baseInput); - assertThat(tracking.baseFor(raw)).isNull(); - assertThat(tracking.getUnmatchedBases()).containsOnly(base); - } - - @Test public void line_hash_has_greater_priority_than_line() { FakeInput baseInput = new FakeInput("H1", "H2", "H3"); Issue base1 = baseInput.createIssueOnLine(1, RULE_SYSTEM_PRINT, "msg"); @@ -369,6 +358,65 @@ public class TrackerTest { assertThat(tracking.getUnmatchedBases()).containsOnly(base2); } + @Test + public void move_manual_issue_to_line_with_same_hash() { + FakeInput baseInput = new FakeInput("H1", "H2"); + Issue issue = baseInput.createIssueOnLine(1, RULE_MANUAL, "message"); + FakeInput rawInput = new FakeInput("H3", "H4", "H1"); + + Tracking<Issue, Issue> tracking = tracker.track(rawInput, baseInput); + + assertThat(tracking.getUnmatchedBases()).isEmpty(); + Multimap<Integer, Issue> openManualIssues = tracking.getOpenManualIssuesByLine(); + assertThat(openManualIssues.keySet()).containsOnly(3); + assertThat(Iterables.getOnlyElement(openManualIssues.get(3))).isSameAs(issue); + } + + @Test + public void do_not_move_manual_issue_if_line_hash_not_found_in_raw() { + FakeInput baseInput = new FakeInput("H1", "H2"); + Issue issue = baseInput.createIssueOnLine(1, RULE_MANUAL, "message"); + FakeInput rawInput = new FakeInput("H3", "H4", "H5"); + + Tracking<Issue, Issue> tracking = tracker.track(rawInput, baseInput); + + assertThat(tracking.getUnmatchedBases()).isEmpty(); + Multimap<Integer, Issue> openManualIssues = tracking.getOpenManualIssuesByLine(); + assertThat(openManualIssues.keySet()).containsOnly(1); + assertThat(Iterables.getOnlyElement(openManualIssues.get(1))).isSameAs(issue); + } + + @Test + public void do_not_match_manual_issue_if_hash_and_line_do_not_exist() { + // manual issue is on line 3 (hash H3) but this hash does not exist + // anymore nor the line 3. + FakeInput baseInput = new FakeInput("H1", "H2", "H3"); + Issue issue = baseInput.createIssueOnLine(3, RULE_MANUAL, "message"); + FakeInput rawInput = new FakeInput("H4"); + + Tracking<Issue, Issue> tracking = tracker.track(rawInput, baseInput); + + assertThat(tracking.getUnmatchedBases()).containsOnly(issue); + assertThat(tracking.getOpenManualIssuesByLine().isEmpty()).isTrue(); + } + + @Test + @Ignore("not implemented yet") + public void move_to_closest_line_if_manual_issue_matches_multiple_hashes() { + // manual issue is on line 3 (hash H3) but this hash does not exist + // anymore nor the line 3. + FakeInput baseInput = new FakeInput("H1", "H2"); + Issue issue = baseInput.createIssueOnLine(1, RULE_MANUAL, "message"); + FakeInput rawInput = new FakeInput("H1", "H3", "H1"); + + Tracking<Issue, Issue> tracking = tracker.track(rawInput, baseInput); + + assertThat(tracking.getUnmatchedBases()).isEmpty(); + Multimap<Integer, Issue> openManualIssues = tracking.getOpenManualIssuesByLine(); + assertThat(openManualIssues.keySet()).containsOnly(1); + assertThat(Iterables.getOnlyElement(openManualIssues.get(1))).isSameAs(issue); + } + private static class Issue implements Trackable { private final RuleKey ruleKey; private final Integer line; diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java index ad5b9bef6e8..5ea870b09fb 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java @@ -146,6 +146,7 @@ public class IssueWorkflowTest { DefaultIssue issue = new DefaultIssue() .setKey("ABCDE") + .setRuleKey(RuleKey.of("js", "S001")) .setResolution(Issue.RESOLUTION_FIXED) .setStatus(Issue.STATUS_RESOLVED) .setNew(false) @@ -269,12 +270,10 @@ public class IssueWorkflowTest { @Test public void manual_issues_be_resolved_then_closed() { - // Manual issue because of reporter DefaultIssue issue = new DefaultIssue() .setKey("ABCDE") .setStatus(Issue.STATUS_OPEN) - .setRuleKey(RuleKey.of("manual", "Performance")) - .setReporter("simon"); + .setRuleKey(RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "Performance")); workflow.start(); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetClosedTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetClosedTest.java index 3418cd9d1d3..f288fcf4cfb 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetClosedTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetClosedTest.java @@ -23,9 +23,10 @@ import org.junit.Test; import org.sonar.api.issue.Issue; import org.sonar.core.issue.DefaultIssue; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.sonar.core.issue.workflow.SetClosed.INSTANCE; public class SetClosedTest { @@ -49,19 +50,6 @@ public class SetClosedTest { } @Test - public void should_fail_if_issue_is_not_resolved() { - Issue issue = new DefaultIssue().setBeingClosed(false); - when(context.issue()).thenReturn(issue); - try { - INSTANCE.execute(context); - fail(); - } catch (IllegalStateException e) { - assertThat(e.getMessage()).contains("Issue is still open"); - verify(context, never()).setResolution(anyString()); - } - } - - @Test public void line_number_must_be_unset() { Issue issue = new DefaultIssue().setBeingClosed(true).setLine(10); when(context.issue()).thenReturn(issue); diff --git a/sonar-core/src/test/java/org/sonar/core/rule/RuleKeyFunctionsTest.java b/sonar-core/src/test/java/org/sonar/core/rule/RuleKeyFunctionsTest.java new file mode 100644 index 00000000000..cf430a18265 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/rule/RuleKeyFunctionsTest.java @@ -0,0 +1,46 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.core.rule; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import org.junit.Test; +import org.sonar.api.rule.RuleKey; +import org.sonar.test.TestUtils; + +import static com.google.common.collect.FluentIterable.from; +import static org.assertj.core.api.Assertions.assertThat; + +public class RuleKeyFunctionsTest { + + @Test + public void stringToRuleKey() throws Exception { + Collection<String> strings = Arrays.asList("js:S001", "java:S002"); + List<RuleKey> keys = from(strings).transform(RuleKeyFunctions.stringToRuleKey()).toList(); + + assertThat(keys).containsExactly(RuleKey.of("js", "S001"), RuleKey.of("java", "S002")); + } + + @Test + public void on_static_methods() throws Exception { + assertThat(TestUtils.hasOnlyPrivateConstructors(RuleKeyFunctions.class)).isTrue(); + } +} diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/selectChangelogOfUnresolvedIssuesByComponent.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/selectChangelogOfNonClosedIssuesByComponent.xml index 966f7c31cac..d809d01a49d 100644 --- a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/selectChangelogOfUnresolvedIssuesByComponent.xml +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/selectChangelogOfNonClosedIssuesByComponent.xml @@ -1,6 +1,6 @@ <dataset> - <!-- Unresolved --> + <!-- Unresolved. To be included --> <issues id="1" kee="UNRESOLVED_ON_FILE_1" @@ -52,7 +52,7 @@ issue_change_creation_date="[null]" /> - <!-- Resolved: to be ignored --> + <!-- Resolved but not closed. To be included --> <issues id="2" kee="RESOLVED_ON_FILE_1" @@ -90,9 +90,47 @@ issue_change_creation_date="1410213600000" /> + <!-- Closed. To be excluded --> + <issues + id="3" + kee="CLOSED_ON_FILE_1" + component_uuid="FILE_1" + project_uuid="PROJECT_1" + resolution="FIXED" + status="CLOSED" + rule_id="501" + severity="MAJOR" + manual_severity="[false]" + message="[null]" + line="120" + effort_to_fix="[null]" + checksum="[null]" + reporter="[null]" + assignee="user" + author_login="[null]" + issue_attributes="[null]" + issue_creation_date="1366063200000" + issue_update_date="1366063200000" + issue_close_date="1366063200000" + created_at="1400000000000" + updated_at="[null]" + /> + + <issue_changes + id="104" + kee="104" + issue_key="CLOSED_ON_FILE_1" + user_login="arthur" + change_type="diff" + change_data="severity=MAJOR|BLOCKER" + created_at="1410213600000" + updated_at="1410213600000" + issue_change_creation_date="1410213600000" + /> + <!-- Unresolved on other file --> <issues - id="3" + id="4" kee="UNRESOLVED_ON_FILE_2" component_uuid="FILE_2" project_uuid="PROJECT_1" @@ -118,8 +156,8 @@ <!-- diff --> <issue_changes - id="104" - kee="104" + id="105" + kee="105" issue_key="UNRESOLVED_ON_FILE_2" user_login="arthur" change_type="diff" @@ -131,8 +169,8 @@ <!-- comment --> <issue_changes - id="105" - kee="105" + id="106" + kee="106" issue_key="UNRESOLVED_ON_FILE_2" user_login="arthur" change_type="comment" |