Browse Source

SONAR-4309 Support concurrent modifications on issues made by batch and simultaneously by user

tags/3.6
Simon Brandhof 11 years ago
parent
commit
b3070fe1c1

+ 9
- 0
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java View File

@@ -19,6 +19,7 @@
*/
package org.sonar.plugins.core.issue;

import org.apache.commons.lang.time.DateUtils;
import org.apache.ibatis.session.ResultContext;
import org.apache.ibatis.session.ResultHandler;
import org.sonar.api.batch.Sensor;
@@ -27,6 +28,9 @@ import org.sonar.api.resources.Project;
import org.sonar.core.issue.db.IssueDao;
import org.sonar.core.issue.db.IssueDto;

import java.util.Calendar;
import java.util.Date;

/**
* Load all the issues referenced during the previous scan.
*/
@@ -47,10 +51,15 @@ public class InitialOpenIssuesSensor implements Sensor {

@Override
public void analyse(Project project, SensorContext context) {
// Adding one second is a hack for resolving conflicts with concurrent user
// changes during issue persistence
final Date now = DateUtils.addSeconds(DateUtils.truncate(new Date(), Calendar.MILLISECOND), 1);

issueDao.selectNonClosedIssuesByModule(project.getId(), new ResultHandler() {
@Override
public void handleResult(ResultContext rc) {
IssueDto dto = (IssueDto) rc.getResultObject();
dto.setSelectedAt(now);
initialOpenIssuesStack.addIssue(dto);
}
});

+ 1
- 0
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java View File

@@ -133,6 +133,7 @@ public class IssueTrackingDecorator implements Decorator {
issue.setNew(false);
issue.setEndOfLife(false);
issue.setOnDisabledRule(false);
issue.setSelectedAt(ref.getSelectedAt());

// fields to update with old values
issue.setActionPlanKey(ref.getActionPlanKey());

+ 18
- 2
sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java View File

@@ -29,7 +29,6 @@ import org.sonar.api.utils.KeyValueFormat;

import javax.annotation.CheckForNull;
import javax.annotation.Nullable;

import java.io.Serializable;
import java.util.Date;

@@ -66,6 +65,11 @@ public final class IssueDto implements Serializable {
private Date createdAt;
private Date updatedAt;

/**
* Temporary date used only during scan
*/
private Date selectedAt;

// joins
private String ruleKey;
private String ruleRepo;
@@ -305,6 +309,16 @@ public final class IssueDto implements Serializable {
return rootComponentKey;
}

@CheckForNull
public Date getSelectedAt() {
return selectedAt;
}

public IssueDto setSelectedAt(Date d) {
this.selectedAt = d;
return this;
}

/**
* Only for unit tests
*/
@@ -357,7 +371,7 @@ public final class IssueDto implements Serializable {
.setIssueCreationDate(issue.creationDate())
.setIssueCloseDate(issue.closeDate())
.setIssueUpdateDate(issue.updateDate())
.setSelectedAt(issue.selectedAt())
.setCreatedAt(now)
.setUpdatedAt(now);
}
@@ -382,6 +396,7 @@ public final class IssueDto implements Serializable {
.setIssueCreationDate(issue.creationDate())
.setIssueCloseDate(issue.closeDate())
.setIssueUpdateDate(issue.updateDate())
.setSelectedAt(issue.selectedAt())
.setUpdatedAt(now);
}

@@ -407,6 +422,7 @@ public final class IssueDto implements Serializable {
issue.setCreationDate(issueCreationDate);
issue.setCloseDate(issueCloseDate);
issue.setUpdateDate(issueUpdateDate);
issue.setSelectedAt(selectedAt);
return issue;
}
}

+ 1
- 0
sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java View File

@@ -40,4 +40,5 @@ public interface IssueMapper {

int update(IssueDto issue);

int updateIfBeforeSelectedDate(IssueDto issue);
}

+ 28
- 16
sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java View File

@@ -19,7 +19,6 @@
*/
package org.sonar.core.issue.db;

import com.google.common.collect.Lists;
import org.apache.ibatis.session.SqlSession;
import org.sonar.api.issue.Issue;
import org.sonar.api.issue.IssueComment;
@@ -32,12 +31,12 @@ import org.sonar.core.persistence.MyBatis;

import java.util.Arrays;
import java.util.Date;
import java.util.List;

public abstract class IssueStorage {

private final MyBatis mybatis;
private final RuleFinder ruleFinder;
private final UpdateConflictResolver conflictResolver = new UpdateConflictResolver();

protected IssueStorage(MyBatis mybatis, RuleFinder ruleFinder) {
this.mybatis = mybatis;
@@ -49,37 +48,50 @@ public abstract class IssueStorage {
}

public void save(Iterable<DefaultIssue> issues) {
SqlSession session = mybatis.openBatchSession();
SqlSession session = mybatis.openSession();
IssueMapper issueMapper = session.getMapper(IssueMapper.class);
IssueChangeMapper issueChangeMapper = session.getMapper(IssueChangeMapper.class);
Date now = new Date();
try {
List<DefaultIssue> conflicts = Lists.newArrayList();
for (DefaultIssue issue : issues) {
if (issue.isNew()) {
long componentId = componentId(issue);
long projectId = projectId(issue);
int ruleId = ruleId(issue);
IssueDto dto = IssueDto.toDtoForInsert(issue, componentId, projectId, ruleId, now);
issueMapper.insert(dto);

insert(issueMapper, now, issue);
} else if (issue.isChanged()) {
IssueDto dto = IssueDto.toDtoForUpdate(issue, now);
int count = issueMapper.update(dto);
if (count < 1) {
conflicts.add(issue);
}
update(issueMapper, now, issue);
}
insertChanges(issueChangeMapper, issue);

}
session.commit();
// TODO log and fix conflicts
} finally {
MyBatis.closeQuietly(session);
}
}

private void insert(IssueMapper issueMapper, Date now, DefaultIssue issue) {
long componentId = componentId(issue);
long projectId = projectId(issue);
int ruleId = ruleId(issue);
IssueDto dto = IssueDto.toDtoForInsert(issue, componentId, projectId, ruleId, now);
issueMapper.insert(dto);
}

private void update(IssueMapper issueMapper, Date now, DefaultIssue issue) {
IssueDto dto = IssueDto.toDtoForUpdate(issue, now);
if (Issue.STATUS_CLOSED.equals(issue.status()) || issue.selectedAt() == null) {
// Issue is closed by scan or changed by end-user
issueMapper.update(dto);

} else {
int count = issueMapper.updateIfBeforeSelectedDate(dto);
if (count == 0) {
// End-user and scan changed the issue at the same time.
// See https://jira.codehaus.org/browse/SONAR-4309
conflictResolver.resolve(issue, issueMapper);
}
}
}

private void insertChanges(IssueChangeMapper mapper, DefaultIssue issue) {
for (IssueComment comment : issue.comments()) {
DefaultIssueComment c = (DefaultIssueComment) comment;

+ 85
- 0
sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java View File

@@ -0,0 +1,85 @@
/*
* 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.core.issue.db;

import com.google.common.annotations.VisibleForTesting;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.issue.internal.DefaultIssue;

import java.util.Date;

/**
* See https://jira.codehaus.org/browse/SONAR-4309
*
* @since 3.6
*/
class UpdateConflictResolver {

private final Logger LOG = LoggerFactory.getLogger(IssueStorage.class);

public void resolve(DefaultIssue issue, IssueMapper mapper) {
LOG.debug("Resolve conflict on issue " + issue.key());

IssueDto dbIssue = mapper.selectByKey(issue.key());
if (dbIssue != null) {
mergeFields(dbIssue, issue);
mapper.update(IssueDto.toDtoForUpdate(issue, new Date()));
}
}

@VisibleForTesting
void mergeFields(IssueDto dbIssue, DefaultIssue issue) {
resolveAssignee(dbIssue, issue);
resolvePlan(dbIssue, issue);
resolveSeverity(dbIssue, issue);
resolveEffortToFix(dbIssue, issue);
resolveResolution(dbIssue, issue);
resolveStatus(dbIssue, issue);
}

private void resolveStatus(IssueDto dbIssue, DefaultIssue issue) {
issue.setStatus(dbIssue.getStatus());
}

private void resolveResolution(IssueDto dbIssue, DefaultIssue issue) {
issue.setResolution(dbIssue.getResolution());
}

private void resolveEffortToFix(IssueDto dbIssue, DefaultIssue issue) {
issue.setEffortToFix(dbIssue.getEffortToFix());
}

private void resolveSeverity(IssueDto dbIssue, DefaultIssue issue) {
if (dbIssue.isManualSeverity()) {
issue.setManualSeverity(true);
issue.setSeverity(dbIssue.getSeverity());
}
// else keep severity as declared in quality profile
}

private void resolvePlan(IssueDto dbIssue, DefaultIssue issue) {
issue.setActionPlanKey(dbIssue.getActionPlanKey());
}

private void resolveAssignee(IssueDto dbIssue, DefaultIssue issue) {
issue.setAssignee(dbIssue.getAssignee());
}
}

+ 25
- 0
sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml View File

@@ -111,6 +111,31 @@
where kee = #{kee}
</update>

<!--
IMPORTANT - invariant columns can't be updated. See IssueDto#toDtoForUpdate()
-->
<update id="updateIfBeforeSelectedDate" parameterType="Issue">
update issues set
action_plan_key=#{actionPlanKey},
severity=#{severity},
manual_severity=#{manualSeverity},
message=#{message},
line=#{line},
effort_to_fix=#{effortToFix},
status=#{status},
resolution=#{resolution},
checksum=#{checksum},
reporter=#{reporter},
assignee=#{assignee},
author_login=#{authorLogin},
issue_attributes=#{issueAttributes},
issue_creation_date=#{issueCreationDate},
issue_update_date=#{issueUpdateDate},
issue_close_date=#{issueCloseDate},
updated_at=#{updatedAt}
where kee = #{kee} and updated_at &lt;= #{selectedAt}
</update>

<select id="selectByKey" parameterType="String" resultType="Issue">
select
<include refid="issueColumns"/>

+ 78
- 0
sonar-core/src/test/java/org/sonar/core/issue/db/IssueMapperTest.java View File

@@ -25,6 +25,10 @@ import org.junit.Test;
import org.sonar.api.utils.DateUtils;
import org.sonar.core.persistence.AbstractDaoTestCase;

import java.util.Date;

import static org.fest.assertions.Assertions.assertThat;

public class IssueMapperTest extends AbstractDaoTestCase {

SqlSession session;
@@ -101,4 +105,78 @@ public class IssueMapperTest extends AbstractDaoTestCase {

checkTables("testUpdate", new String[]{"id"}, "issues");
}

@Test
public void updateBeforeSelectedDate_without_conflict() throws Exception {
setupData("testUpdate");

IssueDto dto = new IssueDto();
dto.setComponentId(123l);
dto.setRootComponentId(100l);
dto.setRuleId(200);
dto.setKee("ABCDE");
dto.setLine(500);
dto.setEffortToFix(3.14);
dto.setResolution("FIXED");
dto.setStatus("RESOLVED");
dto.setSeverity("BLOCKER");
dto.setReporter("emmerik");
dto.setAuthorLogin("morgan");
dto.setAssignee("karadoc");
dto.setActionPlanKey("current_sprint");
dto.setIssueAttributes("JIRA=FOO-1234");
dto.setChecksum("123456789");
dto.setMessage("the message");
dto.setIssueCreationDate(DateUtils.parseDate("2013-05-18"));
dto.setIssueUpdateDate(DateUtils.parseDate("2013-05-19"));
dto.setIssueCloseDate(DateUtils.parseDate("2013-05-20"));
dto.setCreatedAt(DateUtils.parseDate("2013-05-21"));
dto.setUpdatedAt(DateUtils.parseDate("2013-05-22"));

// selected after last update -> ok
dto.setSelectedAt(DateUtils.parseDate("2015-01-01"));

int count = mapper.updateIfBeforeSelectedDate(dto);
assertThat(count).isEqualTo(1);
session.commit();

checkTables("testUpdate", new String[]{"id"}, "issues");
}

@Test
public void updateBeforeSelectedDate_with_conflict() throws Exception {
setupData("updateBeforeSelectedDate_with_conflict");

IssueDto dto = new IssueDto();
dto.setComponentId(123l);
dto.setRootComponentId(100l);
dto.setRuleId(200);
dto.setKee("ABCDE");
dto.setLine(500);
dto.setEffortToFix(3.14);
dto.setResolution("FIXED");
dto.setStatus("RESOLVED");
dto.setSeverity("BLOCKER");
dto.setReporter("emmerik");
dto.setAuthorLogin("morgan");
dto.setAssignee("karadoc");
dto.setActionPlanKey("current_sprint");
dto.setIssueAttributes("JIRA=FOO-1234");
dto.setChecksum("123456789");
dto.setMessage("the message");
dto.setIssueCreationDate(DateUtils.parseDate("2013-05-18"));
dto.setIssueUpdateDate(DateUtils.parseDate("2013-05-19"));
dto.setIssueCloseDate(DateUtils.parseDate("2013-05-20"));
dto.setCreatedAt(DateUtils.parseDate("2013-05-21"));
dto.setUpdatedAt(DateUtils.parseDate("2013-05-22"));

// selected before last update -> ko
dto.setSelectedAt(DateUtils.parseDate("2009-01-01"));

int count = mapper.updateIfBeforeSelectedDate(dto);
assertThat(count).isEqualTo(0);
session.commit();

checkTables("updateBeforeSelectedDate_with_conflict", new String[]{"id"}, "issues");
}
}

+ 149
- 0
sonar-core/src/test/java/org/sonar/core/issue/db/UpdateConflictResolverTest.java View File

@@ -0,0 +1,149 @@
/*
* 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.core.issue.db;

import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.sonar.api.issue.Issue;
import org.sonar.api.issue.internal.DefaultIssue;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.Severity;
import org.sonar.api.utils.DateUtils;

import static org.fest.assertions.Assertions.assertThat;
import static org.mockito.Mockito.*;

public class UpdateConflictResolverTest {

@Test
public void should_reload_issue_and_resolve_conflict() throws Exception {
DefaultIssue issue = new DefaultIssue()
.setKey("ABCDE")
.setRuleKey(RuleKey.of("squid", "AvoidCycles"))
.setComponentKey("struts:org.apache.struts.Action")
.setNew(false)
.setStatus(Issue.STATUS_OPEN);

// Issue as seen and changed by end-user
IssueMapper mapper = mock(IssueMapper.class);
when(mapper.selectByKey("ABCDE")).thenReturn(
new IssueDto()
.setKee("ABCDE")
.setRuleId(10)
.setRuleKey_unit_test_only("squid", "AvoidCycles")
.setComponentId(100L)
.setComponentKey_unit_test_only("struts:org.apache.struts.Action")
.setLine(10)
.setStatus(Issue.STATUS_OPEN)

// field changed by user
.setAssignee("arthur")
);

new UpdateConflictResolver().resolve(issue, mapper);

ArgumentCaptor<IssueDto> argument = ArgumentCaptor.forClass(IssueDto.class);
verify(mapper).update(argument.capture());
IssueDto updatedIssue = argument.getValue();
assertThat(updatedIssue.getKee()).isEqualTo("ABCDE");
assertThat(updatedIssue.getAssignee()).isEqualTo("arthur");
}

@Test
public void should_keep_changes_made_by_user() throws Exception {
DefaultIssue issue = new DefaultIssue()
.setKey("ABCDE")
.setRuleKey(RuleKey.of("squid", "AvoidCycles"))
.setComponentKey("struts:org.apache.struts.Action")
.setNew(false);

// Before starting scan
issue.setAssignee(null);
issue.setActionPlanKey("PLAN-1");
issue.setCreationDate(DateUtils.parseDate("2012-01-01"));
issue.setUpdateDate(DateUtils.parseDate("2012-02-02"));

// Changed by scan
issue.setLine(200);
issue.setSeverity(Severity.BLOCKER);
issue.setManualSeverity(false);
issue.setAuthorLogin("simon");
issue.setChecksum("CHECKSUM-ABCDE");
issue.setResolution(null);
issue.setStatus(Issue.STATUS_REOPENED);

// Issue as seen and changed by end-user
IssueDto dbIssue = new IssueDto()
.setKee("ABCDE")
.setRuleId(10)
.setRuleKey_unit_test_only("squid", "AvoidCycles")
.setComponentId(100L)
.setComponentKey_unit_test_only("struts:org.apache.struts.Action")
.setLine(10)
.setResolution(Issue.RESOLUTION_FALSE_POSITIVE)
.setStatus(Issue.STATUS_RESOLVED)
.setAssignee("arthur")
.setActionPlanKey("PLAN-2")
.setSeverity(Severity.MAJOR)
.setManualSeverity(false);

new UpdateConflictResolver().mergeFields(dbIssue, issue);

assertThat(issue.key()).isEqualTo("ABCDE");
assertThat(issue.componentKey()).isEqualTo("struts:org.apache.struts.Action");

// Scan wins on :
assertThat(issue.line()).isEqualTo(200);
assertThat(issue.severity()).isEqualTo(Severity.BLOCKER);
assertThat(issue.manualSeverity()).isFalse();

// User wins on :
assertThat(issue.assignee()).isEqualTo("arthur");
assertThat(issue.resolution()).isEqualTo(Issue.RESOLUTION_FALSE_POSITIVE);
assertThat(issue.status()).isEqualTo(Issue.STATUS_RESOLVED);
assertThat(issue.actionPlanKey()).isEqualTo("PLAN-2");
}

@Test
public void severity_changed_by_user_should_be_kept() throws Exception {
DefaultIssue issue = new DefaultIssue()
.setKey("ABCDE")
.setRuleKey(RuleKey.of("squid", "AvoidCycles"))
.setComponentKey("struts:org.apache.struts.Action")
.setNew(false)
.setStatus(Issue.STATUS_OPEN);

// Changed by scan
issue.setSeverity(Severity.BLOCKER);
issue.setManualSeverity(false);

// Issue as seen and changed by end-user
IssueDto dbIssue = new IssueDto()
.setKee("ABCDE")
.setStatus(Issue.STATUS_OPEN)
.setSeverity(Severity.INFO)
.setManualSeverity(true);

new UpdateConflictResolver().mergeFields(dbIssue, issue);

assertThat(issue.severity()).isEqualTo(Severity.INFO);
assertThat(issue.manualSeverity()).isTrue();
}
}

+ 1
- 1
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/testUpdate.xml View File

@@ -21,7 +21,7 @@
issue_update_date="[null]"
issue_close_date="[null]"
created_at="[null]"
updated_at="[null]"
updated_at="2009-01-01"
action_plan_key="[null]"
/>
</dataset>

+ 28
- 0
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/updateBeforeSelectedDate_with_conflict-result.xml View File

@@ -0,0 +1,28 @@
<dataset>
<!-- not updated -->
<issues
id="100"
kee="ABCDE"
component_id="123"
root_component_id="100"
rule_id="200"
severity="INFO"
manual_severity="[false]"
message="old"
line="[null]"
effort_to_fix="[null]"
status="OPEN"
resolution="[null]"
checksum="[null]"
reporter="[null]"
author_login="[null]"
assignee="[null]"
issue_attributes="[null]"
issue_creation_date="[null]"
issue_update_date="[null]"
issue_close_date="[null]"
created_at="[null]"
updated_at="2013-06-01"
action_plan_key="[null]"
/>
</dataset>

+ 27
- 0
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/updateBeforeSelectedDate_with_conflict.xml View File

@@ -0,0 +1,27 @@
<dataset>
<issues
id="100"
kee="ABCDE"
component_id="123"
root_component_id="100"
rule_id="200"
severity="INFO"
manual_severity="[false]"
message="old"
line="[null]"
effort_to_fix="[null]"
status="OPEN"
resolution="[null]"
checksum="[null]"
reporter="[null]"
author_login="[null]"
assignee="[null]"
issue_attributes="[null]"
issue_creation_date="[null]"
issue_update_date="[null]"
issue_close_date="[null]"
created_at="[null]"
updated_at="2013-06-01"
action_plan_key="[null]"
/>
</dataset>

+ 14
- 0
sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/DefaultIssue.java View File

@@ -89,6 +89,9 @@ public class DefaultIssue implements Issue {
// true if some fields have been changed since the previous scan
private boolean isChanged = false;

// Date when issue was loaded from db (only when isNew=false)
private Date selectedAt;

public String key() {
return key;
}
@@ -380,6 +383,15 @@ public class DefaultIssue implements Issue {
return Objects.firstNonNull(comments, Collections.<IssueComment>emptyList());
}

@CheckForNull
public Date selectedAt() {
return selectedAt;
}

public void setSelectedAt(@Nullable Date d) {
this.selectedAt = d;
}

@Override
public boolean equals(Object o) {
if (this == o) {
@@ -404,4 +416,6 @@ public class DefaultIssue implements Issue {
public String toString() {
return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE);
}


}

Loading…
Cancel
Save