From b3070fe1c1fc8c15cf8ba5e0433fec598572bdc4 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 4 Jun 2013 19:04:44 +0200 Subject: [PATCH] SONAR-4309 Support concurrent modifications on issues made by batch and simultaneously by user --- .../core/issue/InitialOpenIssuesSensor.java | 9 ++ .../core/issue/IssueTrackingDecorator.java | 1 + .../org/sonar/core/issue/db/IssueDto.java | 20 ++- .../org/sonar/core/issue/db/IssueMapper.java | 1 + .../org/sonar/core/issue/db/IssueStorage.java | 44 ++++-- .../core/issue/db/UpdateConflictResolver.java | 85 ++++++++++ .../org/sonar/core/issue/db/IssueMapper.xml | 25 +++ .../sonar/core/issue/db/IssueMapperTest.java | 78 +++++++++ .../issue/db/UpdateConflictResolverTest.java | 149 ++++++++++++++++++ .../issue/db/IssueMapperTest/testUpdate.xml | 2 +- ...eforeSelectedDate_with_conflict-result.xml | 28 ++++ ...updateBeforeSelectedDate_with_conflict.xml | 27 ++++ .../api/issue/internal/DefaultIssue.java | 14 ++ 13 files changed, 464 insertions(+), 19 deletions(-) create mode 100644 sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java create mode 100644 sonar-core/src/test/java/org/sonar/core/issue/db/UpdateConflictResolverTest.java create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/updateBeforeSelectedDate_with_conflict-result.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/updateBeforeSelectedDate_with_conflict.xml diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java index 9796c39293b..cff5ec73080 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java @@ -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); } }); diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 1728af0ffd3..539dd16224f 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -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()); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java index 412d4deb26f..b0d87a27c9b 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java @@ -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; } } 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 d0f81df029d..42c9b091340 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 @@ -40,4 +40,5 @@ public interface IssueMapper { int update(IssueDto issue); + int updateIfBeforeSelectedDate(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 b8287a83655..bf4050312c0 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 @@ -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 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 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; 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 new file mode 100644 index 00000000000..5ebe79bf908 --- /dev/null +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java @@ -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()); + } +} 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 855800149a5..e52ab7b8648 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 @@ -111,6 +111,31 @@ where kee = #{kee} + + + 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 <= #{selectedAt} + +