Skip to content

Commit 28c7193

Browse files
Ilya VoroninSpace Team
Ilya Voronin
authored and
Space Team
committed
TW-79981 don't publish commented/marked_as_successful status if newer final status was already published for the revision
Merge-request: TC-MR-8065 Merged-by: Ilya Voronin <[email protected]>
1 parent 6263cfd commit 28c7193

File tree

18 files changed

+214
-55
lines changed

18 files changed

+214
-55
lines changed

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/BaseCommitStatusPublisher.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import jetbrains.buildServer.util.StringUtil;
2525
import jetbrains.buildServer.vcs.SVcsModification;
2626
import jetbrains.buildServer.vcs.VcsRoot;
27+
import org.apache.commons.lang.math.NumberUtils;
2728
import org.jetbrains.annotations.NotNull;
2829
import org.jetbrains.annotations.Nullable;
2930

@@ -39,6 +40,7 @@ public abstract class BaseCommitStatusPublisher implements CommitStatusPublisher
3940
protected SBuildType myBuildType;
4041
private final String myBuildFeatureId;
4142
private final CommitStatusPublisherSettings mySettings;
43+
private static final String BUILD_ID_URL_PARAM = "buildId=";
4244

4345
protected BaseCommitStatusPublisher(@NotNull CommitStatusPublisherSettings settings,
4446
@NotNull SBuildType buildType,@NotNull String buildFeatureId,
@@ -180,4 +182,23 @@ protected String getViewUrl(@NotNull BuildPromotion buildPromotion) {
180182
protected String getViewUrl(@NotNull SBuild build) {
181183
return getLinks().getViewResultsUrl(build);
182184
}
185+
186+
protected Long getBuildIdFromViewUrl(@Nullable String url) {
187+
if (url == null) {
188+
return null;
189+
}
190+
int i = url.indexOf(BUILD_ID_URL_PARAM);
191+
if (i == -1) {
192+
return null;
193+
}
194+
i += BUILD_ID_URL_PARAM.length();
195+
196+
StringBuilder idBuilder = new StringBuilder();
197+
while (i < url.length() && Character.isDigit(url.charAt(i))) {
198+
idBuilder.append(url.charAt(i++));
199+
}
200+
201+
Long buildId = NumberUtils.toLong(idBuilder.toString(), -1);
202+
return buildId == -1 ? null : buildId;
203+
}
183204
}

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/CommitStatusPublisher.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,13 @@ public boolean isRetryable() {
120120
return myShouldRetry;
121121
}
122122

123+
/**
124+
* @return true if this event can override status from newer build with status of older build
125+
*/
126+
public boolean canOverrideStatus() {
127+
return this == COMMENTED || this == MARKED_AS_SUCCESSFUL;
128+
}
129+
123130
private enum EventPriority {
124131
FIRST, // the event of this priority will not be accepted if any of the previous events are of the type CONSEQUENT
125132
ANY, // accepted at any time, will not prevent any events to be accepted after it

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/CommitStatusPublisherListener.java‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ public void buildFinished(@NotNull SRunningBuild build) {
257257

258258
final SFinishedBuild finishedBuild = myBuildHistory.findEntry(build.getBuildId());
259259
if (finishedBuild == null) {
260-
LOG.debug("Event: " + Event.FINISHED + ", cannot find finished build for build " + LogUtil.describe(build));
260+
LOG.debug(() -> "Event: " + Event.FINISHED + ", cannot find finished build for build " + LogUtil.describe(build));
261261
return;
262262
}
263263

@@ -280,7 +280,7 @@ public void buildInterrupted(@NotNull SRunningBuild build) {
280280

281281
final SFinishedBuild finishedBuild = myBuildHistory.findEntry(build.getBuildId());
282282
if (finishedBuild == null) {
283-
LOG.debug("Event: " + Event.INTERRUPTED.getName() + ", cannot find finished build for build " + LogUtil.describe(build));
283+
LOG.debug(() -> "Event: " + Event.INTERRUPTED.getName() + ", cannot find finished build for build " + LogUtil.describe(build));
284284
return;
285285
}
286286

@@ -401,7 +401,7 @@ private boolean shouldNotPublish(@Nullable SBuildType buildType, @NotNull BuildR
401401
private boolean shouldPublishQueuedEvent(@NotNull SQueuedBuild queuedBuild) {
402402
final BuildPromotion buildPromotion = queuedBuild.getBuildPromotion();
403403
if (buildPromotion.isPartOfBuildChain() && buildPromotion.getContainingChanges().isEmpty() && buildPromotion.getNumberOfDependedOnMe() != 0) {
404-
LOG.debug(String.format("The build #%s is part of a build chain and has no new changes in it. Queued build status will not be published as Commit Status Publisher suspects that the build can be optimized away.", queuedBuild.getItemId()));
404+
LOG.debug(() -> String.format("The build #%s is part of a build chain and has no new changes in it. Queued build status will not be published as Commit Status Publisher suspects that the build can be optimized away.", queuedBuild.getItemId()));
405405
return false;
406406
}
407407

@@ -479,7 +479,7 @@ private String getTaskIdentity(@NotNull Event event, long id, @Nullable Long del
479479

480480
private void submitTaskForBuild(@NotNull Event event, @NotNull SBuild build, @Nullable Long delay) {
481481
if (!myServerResponsibility.isResponsibleForBuild(build)) {
482-
LOG.debug("Current node is not responsible for build " + LogUtil.describe(build) + ", skip processing event " + event);
482+
LOG.debug(() -> "Current node is not responsible for build " + LogUtil.describe(build) + ", skip processing event " + event);
483483
return;
484484
}
485485

@@ -495,7 +495,7 @@ private void submitTaskForQueuedBuild(@NotNull Event event, @NotNull BuildPromot
495495
private boolean isCurrentRevisionSuitable(Event event, BuildPromotion buildPromotion, BuildRevision revision, CommitStatusPublisher publisher) throws PublisherException {
496496
if (TeamCityProperties.getBooleanOrTrue(CHECK_STATUS_BEFORE_PUBLISHING)) {
497497
RevisionStatus revisionStatus = publisher.getRevisionStatus(buildPromotion, revision);
498-
return revisionStatus == null || revisionStatus.isEventAllowed(event);
498+
return revisionStatus == null || revisionStatus.isEventAllowed(event, buildPromotion.getId());
499499
}
500500
return true;
501501
}
@@ -544,7 +544,7 @@ private void proccessPublishing(Event event, BuildPromotion buildPromotion, Publ
544544
return;
545545
}
546546
Map<String, CommitStatusPublisher> publishers = getPublishers(buildType);
547-
LOG.debug("Event: " + event.getName() + ", build promotion " + LogUtil.describe(buildPromotion) + ", publishers: " + publishers.values());
547+
LOG.debug(() -> "Event: " + event.getName() + ", build promotion " + LogUtil.describe(buildPromotion) + ", publishers: " + publishers.values());
548548
for (CommitStatusPublisher publisher : publishers.values()) {
549549
if (!publisher.isEventSupported(event))
550550
continue;
@@ -611,7 +611,7 @@ private static String getDefaultComment(@NotNull BuildPromotion buildPromotion,
611611
private SBuildType getBuildType(@NotNull Event event, @NotNull SBuild build) {
612612
SBuildType buildType = build.getBuildType();
613613
if (buildType == null)
614-
LOG.debug("Event: " + event.getName() + ", cannot find buildType for build " + LogUtil.describe(build));
614+
LOG.debug(() -> "Event: " + event.getName() + ", cannot find buildType for build " + LogUtil.describe(build));
615615
return buildType;
616616
}
617617

@@ -620,7 +620,7 @@ private SBuildType getBuildType(@NotNull Event event, @NotNull SQueuedBuild buil
620620
try {
621621
return build.getBuildType();
622622
} catch (BuildTypeNotFoundException e) {
623-
LOG.debug("Event: " + event.getName() + ", cannot find buildType for build " + LogUtil.describe(build));
623+
LOG.debug(() -> "Event: " + event.getName() + ", cannot find buildType for build " + LogUtil.describe(build));
624624
return null;
625625
}
626626
}
@@ -650,7 +650,7 @@ private Collection<BuildRevision> getQueuedBuildRevisionForVote(@NotNull BuildTy
650650
if (!((BuildPromotionEx)buildPromotion).isChangeCollectingNeeded(false)) {
651651
return getBuildRevisionForVote(publisher, buildPromotion.getRevisions());
652652
}
653-
LOG.debug("No revision is found for build " + buildPromotion.getBuildTypeExternalId() + ". Queue-related status won't be published");
653+
LOG.debug(() -> "No revision is found for build " + buildPromotion.getBuildTypeExternalId() + ". Queue-related status won't be published");
654654
return Collections.emptyList();
655655
}
656656

@@ -809,8 +809,23 @@ public RetryInfo publish(Event event, BuildRevision revision, CommitStatusPublis
809809

810810
Lock lock = myPublishingLocks.get(revision.getRevision());
811811
lock.lock();
812+
812813
try {
813-
retryInfo = runTask(event, buildPromotion, LogUtil.describe(build), task, publisher, revision, null, lastDelay);
814+
boolean isEventSuitableForRevision = true;
815+
if (event.canOverrideStatus()) {
816+
try {
817+
isEventSuitableForRevision = isCurrentRevisionSuitable(event, buildPromotion, revision, publisher);
818+
} catch (PublisherException e) {
819+
retryInfo = getRetryInfo(e, buildPromotion, event, lastDelay);
820+
LOG.warn("Cannot determine if event \"" + event + "\" can be published for current revision state in VCS. " + retryInfo.message, e);
821+
return retryInfo;
822+
}
823+
}
824+
if (isEventSuitableForRevision) {
825+
retryInfo = runTask(event, buildPromotion, LogUtil.describe(build), task, publisher, revision, null, lastDelay);
826+
} else {
827+
LOG.debug(() -> "Event \"" + event + "\" is not suitable to be published to root \"" + publisher.getVcsRootId() + "\" for revision " + revision.getRevision());
828+
}
814829
} finally {
815830
lock.unlock();
816831
}
@@ -932,7 +947,7 @@ private RetryInfo doPublish(BuildRevision revision, CommitStatusPublisher publis
932947
if (isEventSuitableForRevision) {
933948
retryInfo = runTask(event, buildPromotion, LogUtil.describe(buildPromotion), publishTask, publisher, revision, additionalTaskInfo, lastDelay);
934949
} else {
935-
LOG.debug("Event \"" + event + "\" is not suitable to be published to rooot \"" + publisher.getVcsRootId() + "\" for revision " + revision.getRevision());
950+
LOG.debug(() -> "Event \"" + event + "\" is not suitable to be published to root \"" + publisher.getVcsRootId() + "\" for revision " + revision.getRevision());
936951
}
937952
return retryInfo;
938953
}

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/RevisionStatus.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@ public class RevisionStatus {
1212
private final CommitStatusPublisher.Event myTriggeredEvent;
1313
private final String myDescription;
1414
private final boolean myIsSameBuildType;
15-
16-
public RevisionStatus(@Nullable CommitStatusPublisher.Event triggeredEvent, @Nullable String description, boolean isSameBuildType) {
15+
private final Long myBuildId;
16+
17+
public RevisionStatus(@Nullable CommitStatusPublisher.Event triggeredEvent, @Nullable String description, boolean isSameBuildType, @Nullable Long buildId) {
1718
myTriggeredEvent = triggeredEvent;
1819
myDescription = description;
1920
myIsSameBuildType = isSameBuildType;
21+
myBuildId = buildId;
2022
}
2123

2224
@Nullable
@@ -29,24 +31,30 @@ public String getDescription() {
2931
return myDescription;
3032
}
3133

32-
public boolean isEventAllowed(@NotNull CommitStatusPublisher.Event pendingEvent) {
34+
public boolean isEventAllowed(@NotNull CommitStatusPublisher.Event pendingEvent, long buildId) {
3335
if (myTriggeredEvent == null) {
34-
return true;
36+
if (pendingEvent.canOverrideStatus()) {
37+
return myBuildId == null || buildId >= myBuildId; // we don't want to publish status for older build
38+
} else {
39+
return true;
40+
}
3541
}
42+
3643
switch (pendingEvent) {
3744
case QUEUED:
3845
return myIsSameBuildType && CommitStatusPublisher.Event.QUEUED == myTriggeredEvent;
3946
case REMOVED_FROM_QUEUE:
4047
return myIsSameBuildType && CommitStatusPublisher.Event.QUEUED == myTriggeredEvent;
48+
case COMMENTED:
49+
case MARKED_AS_SUCCESSFUL:
50+
return myBuildId == null || buildId >= myBuildId;
4151
case STARTED:
4252
case FINISHED:
4353
case INTERRUPTED:
4454
case FAILURE_DETECTED:
45-
case COMMENTED:
46-
case MARKED_AS_SUCCESSFUL:
4755
return true;
4856
default:
49-
LOG.info("Unknown Comit Status Publisher event received: \"" + pendingEvent + "\". It will be allowed to be processed");
57+
LOG.info("Unknown Commit Status Publisher event received: \"" + pendingEvent + "\". It will be allowed to be processed");
5058
}
5159
return true;
5260
}

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/bitbucketCloud/BitbucketCloudPublisher.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ RevisionStatus getRevisionStatusForRemovedBuild(@NotNull SQueuedBuild removedBui
144144
}
145145
Event event = getTriggeredEvent(buildStatus);
146146
boolean isSameBuildType = StringUtil.areEqual(removedBuild.getBuildTypeId(), buildStatus.key);
147-
return new RevisionStatus(event, buildStatus.description, isSameBuildType);
147+
return new RevisionStatus(event, buildStatus.description, isSameBuildType, getBuildIdFromViewUrl(buildStatus.url));
148148
}
149149

150150
@Override
@@ -213,7 +213,7 @@ RevisionStatus getRevisionStatus(@NotNull BuildPromotion buildPromotion, @Nullab
213213
}
214214
Event event = getTriggeredEvent(commitStatus);
215215
boolean isSameBuildType = StringUtil.areEqual(buildPromotion.getBuildTypeId(), commitStatus.key);
216-
return new RevisionStatus(event, commitStatus.description, isSameBuildType);
216+
return new RevisionStatus(event, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.url));
217217
}
218218

219219
@Nullable

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GitHubPublisher.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ RevisionStatus getRevisionStatus(@NotNull BuildPromotion buildPromotion, @Nullab
114114
}
115115
Event triggeredEvent = getTriggeredEvent(commitStatus);
116116
boolean isSameBuildType = isSameBuildType(buildPromotion, commitStatus);
117-
return new RevisionStatus(triggeredEvent, commitStatus.description, isSameBuildType);
117+
return new RevisionStatus(triggeredEvent, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.target_url));
118118
}
119119

120120
private boolean isSameBuildType(BuildPromotion buildPromotion, CommitStatus commitStatus) {
@@ -145,7 +145,7 @@ RevisionStatus getRevisionStatusForRemovedBuild(@NotNull SQueuedBuild removedBui
145145
}
146146
Event triggeredEvent = getTriggeredEvent(commitStatus);
147147
boolean isSameBuildType = isSameBuildType(removedBuild.getBuildPromotion(), commitStatus);
148-
return new RevisionStatus(triggeredEvent, commitStatus.description, isSameBuildType);
148+
return new RevisionStatus(triggeredEvent, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.target_url));
149149
}
150150

151151
@Nullable

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/gitlab/GitlabPublisher.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ RevisionStatus getRevisionStatusForRemovedBuild(@NotNull SQueuedBuild removedBui
136136
}
137137
Event event = getTriggeredEvent(commitStatus);
138138
boolean isSameBuildType = StringUtil.areEqual(getBuildName(removedBuild.getBuildPromotion()), commitStatus.name);
139-
return new RevisionStatus(event, commitStatus.description, isSameBuildType);
139+
return new RevisionStatus(event, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.target_url));
140140
}
141141

142142
private String getBuildName(BuildPromotion promotion) {
@@ -189,7 +189,7 @@ RevisionStatus getRevisionStatus(@NotNull BuildPromotion buildPromotion, @Nullab
189189
}
190190
Event event = getTriggeredEvent(commitStatus);
191191
boolean isSameBuildType = StringUtil.areEqual(getBuildName(buildPromotion), commitStatus.name);
192-
return new RevisionStatus(event, commitStatus.description, isSameBuildType);
192+
return new RevisionStatus(event, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.target_url));
193193
}
194194

195195
private String buildRevisionStatusesUrl(@NotNull BuildRevision revision, @Nullable BuildType buildType) throws PublisherException {

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/space/SpacePublisher.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import jetbrains.buildServer.vcs.VcsModification;
3232
import jetbrains.buildServer.vcs.VcsRootInstance;
3333
import jetbrains.buildServer.vcshostings.http.HttpHelper;
34+
import org.apache.commons.lang.math.NumberUtils;
3435
import org.apache.http.HttpHeaders;
3536
import org.apache.http.entity.ContentType;
3637
import org.jetbrains.annotations.NotNull;
@@ -131,7 +132,13 @@ RevisionStatus getRevisionStatusForRemovedBuild(@NotNull SQueuedBuild removedBui
131132
}
132133
Event event = getTriggeredEvent(buildStatus);
133134
boolean isSameBuildType = StringUtil.areEqual(getTaskId(removedBuild.getBuildPromotion()), buildStatus.taskId);
134-
return new RevisionStatus(event, buildStatus.description, isSameBuildType);
135+
return new RevisionStatus(event, buildStatus.description, isSameBuildType, getBuildId(buildStatus));
136+
}
137+
138+
@Nullable
139+
private Long getBuildId(@NotNull SpaceBuildStatusInfo buildStatus) {
140+
Long buildId = NumberUtils.toLong(buildStatus.taskBuildId, -1);
141+
return buildId > -1 ? buildId : getBuildIdFromViewUrl(buildStatus.url);
135142
}
136143

137144
private String getTaskId(BuildPromotion buildPromotion) {
@@ -192,7 +199,7 @@ RevisionStatus getRevisionStatus(@NotNull BuildPromotion buildPromotion, @Nullab
192199
}
193200
Event event = getTriggeredEvent(commitStatus);
194201
boolean isSameBuild = StringUtil.areEqual(getTaskId(buildPromotion), commitStatus.taskId);
195-
return new RevisionStatus(event, commitStatus.description, isSameBuild);
202+
return new RevisionStatus(event, commitStatus.description, isSameBuild, getBuildId(commitStatus));
196203
}
197204

198205
private Event getTriggeredEvent(SpaceBuildStatusInfo commitStatus) {

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/stash/StashPublisher.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import jetbrains.buildServer.vcs.VcsRootInstance;
3636
import jetbrains.buildServer.vcshostings.http.HttpHelper;
3737
import jetbrains.buildServer.vcshostings.http.credentials.HttpCredentials;
38+
import org.apache.commons.lang.math.NumberUtils;
3839
import org.jetbrains.annotations.NotNull;
3940
import org.jetbrains.annotations.Nullable;
4041

@@ -148,7 +149,14 @@ RevisionStatus getRevisionStatusForRemovedBuild(@NotNull SQueuedBuild removedBui
148149
}
149150
Event event = getTriggeredEvent(buildStatus);
150151
boolean isSameBuildType = StringUtil.areEqual(getBuildKey(removedBuild.getBuildPromotion()), buildStatus.key);
151-
return new RevisionStatus(event, buildStatus.description, isSameBuildType);
152+
return new RevisionStatus(event, buildStatus.description, isSameBuildType, getBuildId(buildStatus));
153+
}
154+
155+
156+
@Nullable
157+
private Long getBuildId(@NotNull JsonStashBuildStatus buildStatus) {
158+
Long buildId = NumberUtils.toLong(buildStatus.buildNumber, -1);
159+
return buildId > -1 ? buildId : getBuildIdFromViewUrl(buildStatus.url);
152160
}
153161

154162
private String getBuildKey(BuildPromotion buildPromotion) {
@@ -187,7 +195,7 @@ RevisionStatus getRevisionStatus(@NotNull BuildPromotion buildPromotion, @Nullab
187195
}
188196
Event event = getTriggeredEvent(buildStatus);
189197
boolean isSameBuildType = StringUtil.areEqual(getBuildKey(buildPromotion), buildStatus.key);
190-
return new RevisionStatus(event, buildStatus.description, isSameBuildType);
198+
return new RevisionStatus(event, buildStatus.description, isSameBuildType, getBuildId(buildStatus));
191199
}
192200

193201
private Event getTriggeredEvent(JsonStashBuildStatus buildStatus) {

‎commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/tfs/TfsStatusPublisher.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ RevisionStatus getRevisionStatusForRemovedBuild(@NotNull SQueuedBuild removedBui
148148
if (commitStatus == null) return null;
149149
Event event = getTriggeredEvent(commitStatus);
150150
boolean isSameBuildType = StringUtil.areEqual(getBuildName(removedBuild.getBuildPromotion()), commitStatus.context.name);
151-
return new RevisionStatus(event, commitStatus.description, isSameBuildType);
151+
return new RevisionStatus(event, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.targetUrl));
152152
}
153153

154154
private String getBuildName(BuildPromotion buildPromotion) {
@@ -207,7 +207,7 @@ RevisionStatus getRevisionStatus(@NotNull BuildPromotion buildPromotion, @Nullab
207207
if (commitStatus == null) return null;
208208
Event event = getTriggeredEvent(commitStatus);
209209
boolean isSameBuildType = StringUtil.areEqual(getBuildName(buildPromotion), commitStatus.context.name);
210-
return new RevisionStatus(event, commitStatus.description, isSameBuildType);
210+
return new RevisionStatus(event, commitStatus.description, isSameBuildType, getBuildIdFromViewUrl(commitStatus.targetUrl));
211211
}
212212

213213
private Event getTriggeredEvent(CommitStatus commitStatus) {

0 commit comments

Comments
 (0)