Merge pull request #512 from cqse/ts/38948_prevent_duplicate_uploads

TS-38948 Prevent multiple corverage uploads to the same project+revision

CHANGELOG.md

@@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [feature] _agent_: Prevent uploading coverage to the same project + revision or branch@timestamp when doing multi project upload via git.properties
 
 # 34.0.1
 - [fix] _agent_: Error was reported when the system under test used logback

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.java

@@ -87,9 +87,9 @@ void searchFile(File file, boolean isJarFile) {
  GitPropertiesLocatorUtils.GIT_PROPERTIES_TEAMSCALE_COMMIT_TIME);
  continue;
  }
- uploader.setTeamscaleProjectForRevision(projectAndCommit);
  logger.debug("Found git.properties file in {} and found Teamscale project {} and revision {}", file,
  projectAndCommit.getProject(), projectAndCommit.getCommitInfo());
+ uploader.addTeamscaleProjectAndCommit(file, projectAndCommit);
  }
  } catch (IOException | InvalidGitPropertiesException e) {
  logger.error("Error during asynchronous search for git.properties in {}", file, e);

agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java

@@ -489,9 +489,9 @@ private DelayedTeamscaleMultiProjectUploader createTeamscaleMultiProjectUploader
  DelayedTeamscaleMultiProjectUploader uploader = new DelayedTeamscaleMultiProjectUploader(
  (project, commitInfo) -> {
  if (commitInfo.preferCommitDescriptorOverRevision || StringUtils.isEmpty(commitInfo.revision)) {
- return new TeamscaleUploader(teamscaleServer.withProjectAndCommit(project, commitInfo.commit));
+ return teamscaleServer.withProjectAndCommit(project, commitInfo.commit);
  }
- return new TeamscaleUploader(teamscaleServer.withProjectAndRevision(project, commitInfo.revision));
+ return teamscaleServer.withProjectAndRevision(project, commitInfo.revision);
  });
 
  if (gitPropertiesJar != null) {

agent/src/main/java/com/teamscale/jacoco/agent/upload/teamscale/DelayedTeamscaleMultiProjectUploader.java

@@ -1,10 +1,12 @@
 package com.teamscale.jacoco.agent.upload.teamscale;
 
+import com.teamscale.client.TeamscaleServer;
 import com.teamscale.jacoco.agent.commit_resolution.git_properties.CommitInfo;
 import com.teamscale.jacoco.agent.options.ProjectAndCommit;
 import com.teamscale.jacoco.agent.upload.DelayedMultiUploaderBase;
 import com.teamscale.jacoco.agent.upload.IUploader;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -13,22 +15,41 @@
 /** Wrapper for {@link TeamscaleUploader} that allows to upload the same coverage file to multiple Teamscale projects. */
 public class DelayedTeamscaleMultiProjectUploader extends DelayedMultiUploaderBase implements IUploader {
 
- private final BiFunction<String, CommitInfo, IUploader> uploaderFactory;
- private final List<IUploader> teamscaleUploaders = new ArrayList<>();
+ private final BiFunction<String, CommitInfo, TeamscaleServer> teamscaleServerFactory;
+ private final List<TeamscaleUploader> teamscaleUploaders = new ArrayList<>();
 
- public DelayedTeamscaleMultiProjectUploader(BiFunction<String, CommitInfo, IUploader> uploaderFactory) {
- this.uploaderFactory = uploaderFactory;
+ public DelayedTeamscaleMultiProjectUploader(
+ BiFunction<String, CommitInfo, TeamscaleServer> teamscaleServerFactory) {
+ this.teamscaleServerFactory = teamscaleServerFactory;
  }
+
+ public List<TeamscaleUploader> getTeamscaleUploaders() {
+ return teamscaleUploaders;
+ }
+
+ /**
+ * Adds a teamscale project and commit as a possible new target to upload coverage to. Checks if the project and
+ * commit are already registered as an upload target and will prevent duplicate uploads.
+ */
+ public void addTeamscaleProjectAndCommit(File file, ProjectAndCommit projectAndCommit) {
 
- /** Sets the project and revision detected for the Teamscale project. */
- public void setTeamscaleProjectForRevision(ProjectAndCommit projectAndCommit) {
- IUploader uploader = uploaderFactory.apply(projectAndCommit.getProject(),
+ TeamscaleServer teamscaleServer = teamscaleServerFactory.apply(projectAndCommit.getProject(),
  projectAndCommit.getCommitInfo());
+
+ if (this.teamscaleUploaders.stream().anyMatch(teamscaleUploader ->
+ teamscaleUploader.getTeamscaleServer().hasSameProjectAndCommit(teamscaleServer)
+ )) {
+ logger.debug(
+ "Project and commit in git.properties file {} are already registered as upload target. Coverage will not be uploaded multiple times to the same project {} and commit info {}.",
+ file, projectAndCommit.getProject(), projectAndCommit.getCommitInfo());
+ return;
+ }
+ TeamscaleUploader uploader = new TeamscaleUploader(teamscaleServer);
  teamscaleUploaders.add(uploader);
  }
 
  @Override
  protected Collection<IUploader> getWrappedUploaders() {
- return teamscaleUploaders;
+ return new ArrayList<>(teamscaleUploaders);
  }
 }

agent/src/main/java/com/teamscale/jacoco/agent/upload/teamscale/TeamscaleUploader.java

@@ -1,22 +1,5 @@
 package com.teamscale.jacoco.agent.upload.teamscale;
 
-import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.COMMIT;
-import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.MESSAGE;
-import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.PARTITION;
-import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.PROJECT;
-import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.REPOSITORY;
-import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.REVISION;
-
-import java.io.File;
-import java.io.IOException;
-import java.io.OutputStreamWriter;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.util.Properties;
-
-import org.conqat.lib.commons.filesystem.FileSystemUtils;
-import org.slf4j.Logger;
-
 import com.google.common.base.Strings;
 import com.teamscale.client.CommitDescriptor;
 import com.teamscale.client.EReportFormat;
@@ -29,6 +12,22 @@
 import com.teamscale.jacoco.agent.util.Benchmark;
 import com.teamscale.jacoco.agent.util.LoggingUtils;
 import com.teamscale.report.jacoco.CoverageFile;
+import org.conqat.lib.commons.filesystem.FileSystemUtils;
+import org.slf4j.Logger;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Properties;
+
+import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.COMMIT;
+import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.MESSAGE;
+import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.PARTITION;
+import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.PROJECT;
+import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.REPOSITORY;
+import static com.teamscale.jacoco.agent.upload.teamscale.ETeamscaleServerProperties.REVISION;
 
 /** Uploads XML Coverage to a Teamscale instance. */
 public class TeamscaleUploader implements IUploader, IUploadRetry {
@@ -41,6 +40,10 @@ public class TeamscaleUploader implements IUploader, IUploadRetry {
  /** The logger. */
  private final Logger logger = LoggingUtils.getLogger(this);
 
+ public TeamscaleServer getTeamscaleServer() {
+ return teamscaleServer;
+ }
+
  /** Teamscale server details. */
  private final TeamscaleServer teamscaleServer;
 

agent/src/test/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocatorTest.java

@@ -1,12 +1,16 @@
 package com.teamscale.jacoco.agent.commit_resolution.git_properties;
 
+import com.teamscale.client.CommitDescriptor;
+import com.teamscale.client.TeamscaleServer;
 import com.teamscale.jacoco.agent.options.ProjectAndCommit;
 import com.teamscale.jacoco.agent.upload.teamscale.DelayedTeamscaleMultiProjectUploader;
+import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleUploader;
 import org.junit.jupiter.api.Test;
 
 import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -18,12 +22,38 @@ void testNoErrorIsThrownWhenGitPropertiesFileDoesNotHaveAProject() {
  GitMultiProjectPropertiesLocator locator = new GitMultiProjectPropertiesLocator(
  new DelayedTeamscaleMultiProjectUploader((project, revision) -> {
  projectAndCommits.add(new ProjectAndCommit(project, revision));
- return null;
+ return new TeamscaleServer();
  }), true);
  File jarFile = new File(getClass().getResource("emptyTeamscaleProjectGitProperties").getFile());
  locator.searchFile(jarFile, false);
  assertThat(projectAndCommits.size()).isEqualTo(1);
  assertThat(projectAndCommits.get(0).getProject()).isEqualTo("my-teamscale-project");
  }
 
+ @Test
+ void testNoMultipleUploadsToSameProjectAndRevision() {
+ DelayedTeamscaleMultiProjectUploader delayedTeamscaleMultiProjectUploader = new DelayedTeamscaleMultiProjectUploader(
+ (project, revision) -> {
+ TeamscaleServer server = new TeamscaleServer();
+ server.project = project;
+ server.revision = revision.revision;
+ server.commit = revision.commit;
+ return server;
+ });
+ GitMultiProjectPropertiesLocator locator = new GitMultiProjectPropertiesLocator(
+ delayedTeamscaleMultiProjectUploader, true
+ );
+ File jarFile = new File(getClass().getResource("multiple-same-target-git-properties-folder").getFile());
+ locator.searchFile(jarFile, false);
+ List<TeamscaleServer> teamscaleServers = delayedTeamscaleMultiProjectUploader.getTeamscaleUploaders().stream()
+ .map(TeamscaleUploader::getTeamscaleServer).collect(Collectors.toList());
+ assertThat(teamscaleServers.size()).isEqualTo(2);
+ assertThat(teamscaleServers.get(0).project).isEqualTo("demo2");
+ assertThat(teamscaleServers.get(0).commit).isEqualTo(
+ new CommitDescriptor("master", "1645713803000"));
+ assertThat(teamscaleServers.get(1).project).isEqualTo("demolib");
+ assertThat(teamscaleServers.get(1).revision).isEqualTo(
+ "05b9d066a0c0762be622987de403b5752fa01cc0");
+ }
+
 }

agent/src/test/resources/com/teamscale/jacoco/agent/commit_resolution/git_properties/multiple-same-target-git-properties-folder/WEB-INF/bar/git.properties

@@ -0,0 +1,3 @@
+teamscale.project=demo2
+git.branch=master
+git.commit.time=2022-02-24T15:43:23+0100

agent/src/test/resources/com/teamscale/jacoco/agent/commit_resolution/git_properties/multiple-same-target-git-properties-folder/WEB-INF/classes/git.properties

@@ -0,0 +1,4 @@
+git.commit.id=05b9d066a0c0762be622987de403b5752fa01cc0
+teamscale.project=demolib
+git.branch=master
+git.commit.time=2022-02-24T15:43:23+0100

agent/src/test/resources/com/teamscale/jacoco/agent/commit_resolution/git_properties/multiple-same-target-git-properties-folder/WEB-INF/foo/git.properties

@@ -0,0 +1,3 @@
+teamscale.project=demo2
+git.branch=master
+git.commit.time=2022-02-24T15:43:23+0100

teamscale-client/src/main/java/com/teamscale/client/TeamscaleServer.java

@@ -1,12 +1,12 @@
 package com.teamscale.client;
 
+import okhttp3.HttpUrl;
+
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 
-import okhttp3.HttpUrl;
-
 /** Holds Teamscale server details. */
 public class TeamscaleServer {
 
@@ -133,6 +133,17 @@ public boolean hasCommitOrRevision() {
  return commit != null || revision != null;
  }
 
+ /** Checks if another TeamscaleServer has the same project and revision/commit as this TeamscaleServer instance. */
+ public boolean hasSameProjectAndCommit(TeamscaleServer other) {
+ if (!this.project.equals(other.project)) {
+ return false;
+ }
+ if (this.revision != null) {
+ return this.revision.equals(other.revision);
+ }
+ return this.commit.equals(other.commit);
+ }
+
  @Override
  public String toString() {
  String at;