Skip to content

Commit

Permalink
TS-38561 Use commit descriptor instead of revision if set via teamsca…
Browse files Browse the repository at this point in the history
…le.timestamp
  • Loading branch information
AnonymFx committed May 22, 2024
1 parent 2c808a4 commit 2001810
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ public class CommitInfo {
/** The commit descriptor. */
public CommitDescriptor commit;

public boolean preferCommitDescriptorOverRevision = false;

Check warning on line 15 in agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/CommitInfo.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/CommitInfo.java#L15

Interface comment missing https://cqse.teamscale.io/findings.html#details/teamscale-jacoco-agent?t=ts%2F38561_read_teamscale.timestamp_from_git.properties%3AHEAD&id=3C0D4F0676B62890FB025FA3B3C2D8D1

/** Constructor. */
public CommitInfo(String revision, CommitDescriptor commit) {
this.revision = revision;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,21 @@ public class GitPropertiesLocatorUtils {
public static final String JAR_FILE_ENDING = ".jar";

/**
* Reads the git SHA1 from the given jar file's git.properties and builds a commit descriptor out of it. If no
* git.properties file can be found, returns null.
* Reads the git SHA1 and branch and timestamp from the given jar file's git.properties and builds a commit
* descriptor out of it. If no git.properties file can be found, returns null.
*
* @throws IOException If reading the jar file fails.
* @throws InvalidGitPropertiesException If a git.properties file is found but it is malformed.
*/
public static List<String> getRevisionsFromGitProperties(
public static List<CommitInfo> getCommitInfoFromGitProperties(
File file, boolean isJarFile, boolean recursiveSearch) throws IOException, InvalidGitPropertiesException {
List<Pair<String, Properties>> entriesWithProperties = findGitPropertiesInFile(file, isJarFile,
recursiveSearch);
List<String> result = new ArrayList<>();
List<CommitInfo> result = new ArrayList<>();
for (Pair<String, Properties> entryWithProperties : entriesWithProperties) {
String revision = getCommitInfoFromGitProperties(entryWithProperties.getSecond(),
entryWithProperties.getFirst(), file).revision; // TODO
result.add(revision);
CommitInfo commitInfo = getCommitInfoFromGitProperties(entryWithProperties.getSecond(),
entryWithProperties.getFirst(), file);
result.add(commitInfo);
}
return result;
}
Expand Down Expand Up @@ -179,8 +179,8 @@ private static String extractArtefactUrl(URL jarOrClassFolderUrl) {
}

/**
* Reads the 'teamscale.project' property values and the git SHA1s from all git.properties files contained in the
* provided folder or archive file.
* Reads the 'teamscale.project' property values and the git SHA1s or branch + timestamp from all git.properties
* files contained in the provided folder or archive file.
*
* @throws IOException If reading the jar file fails.
* @throws InvalidGitPropertiesException If a git.properties file is found but it is malformed.
Expand All @@ -191,20 +191,19 @@ public static List<ProjectAndCommit> getProjectRevisionsFromGitProperties(
recursiveSearch);
List<ProjectAndCommit> result = new ArrayList<>();

Check failure on line 192 in agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java#L192

Type depends on `com.teamscale.jacoco.agent.options.ProjectAndCommit`. This violates the architecture specification in `teamscale-jacoco-agent.architecture` https://cqse.teamscale.io/findings.html#details/teamscale-jacoco-agent?t=ts%2F38561_read_teamscale.timestamp_from_git.properties%3AHEAD&id=8B0A0A69891A1E9D2838761A836E5ABB
for (Pair<String, Properties> entryWithProperties : entriesWithProperties) {
String revision = entryWithProperties.getSecond().getProperty(GIT_PROPERTIES_GIT_COMMIT_ID);
if (StringUtils.isEmpty(revision)) {
revision = entryWithProperties.getSecond().getProperty(GIT_PROPERTIES_GIT_COMMIT_ID_FULL);
}
CommitInfo commitInfo = getCommitInfoFromGitProperties(entryWithProperties.getSecond(),
entryWithProperties.getFirst(), file);
String project = entryWithProperties.getSecond().getProperty(GIT_PROPERTIES_TEAMSCALE_PROJECT);
if (StringUtils.isEmpty(revision) && StringUtils.isEmpty(project)) {
if (StringUtils.isEmpty(project)) {
// commitInfo is not empty, corresponding checks are implemented in getCommitInfoForGitProperties
throw new InvalidGitPropertiesException(
"No entry or empty value for both '" + GIT_PROPERTIES_GIT_COMMIT_ID + "'/'" + GIT_PROPERTIES_GIT_COMMIT_ID_FULL +
"' and '" + GIT_PROPERTIES_TEAMSCALE_PROJECT + "' in " + file + "." +
"\nContents of " + GIT_PROPERTIES_FILE_NAME + ": " + entryWithProperties.getSecond()
.toString()
);
}
result.add(new ProjectAndCommit(project, new CommitInfo(revision, null))); // TODO
result.add(new ProjectAndCommit(project, commitInfo));

Check failure on line 206 in agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java#L206

Type depends on `com.teamscale.jacoco.agent.options.ProjectAndCommit`. This violates the architecture specification in `teamscale-jacoco-agent.architecture` https://cqse.teamscale.io/findings.html#details/teamscale-jacoco-agent?t=ts%2F38561_read_teamscale.timestamp_from_git.properties%3AHEAD&id=8B0A0A69891A1E9D2838761A836E5ABB
}
return result;
}
Expand Down Expand Up @@ -365,6 +364,7 @@ public static CommitInfo getCommitInfoFromGitProperties(
commitDescriptor = new CommitDescriptor(gitBranch, gitTimestamp);
}

boolean preferCommitDescriptorOverRevision = false;
String teamscaleTimestampProperty = gitProperties.getProperty(GIT_PROPERTIES_TEAMSCALE_TIMESTAMP);
if (!StringUtils.isEmpty(teamscaleTimestampProperty)) {
String[] split = teamscaleTimestampProperty.split(":");
Expand Down Expand Up @@ -396,6 +396,7 @@ public static CommitInfo getCommitInfoFromGitProperties(
}

commitDescriptor = new CommitDescriptor(branch, epochTimestamp);
preferCommitDescriptorOverRevision = true;
}
}

Expand All @@ -407,6 +408,8 @@ public static CommitInfo getCommitInfoFromGitProperties(
"\nContents of " + GIT_PROPERTIES_FILE_NAME + ":\n" + gitProperties);
}

return new CommitInfo(revision, commitDescriptor);
CommitInfo commitInfo = new CommitInfo(revision, commitDescriptor);
commitInfo.preferCommitDescriptorOverRevision = preferCommitDescriptorOverRevision;
return commitInfo;

Check warning on line 413 in agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java

View check run for this annotation

cqse.teamscale.io / teamscale-findings

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java#L346-L413

Violation of method length threshold (source lines of code) of 30: 63 https://cqse.teamscale.io/findings.html#details/teamscale-jacoco-agent?t=ts%2F38561_read_teamscale.timestamp_from_git.properties%3AHEAD&id=CE6C3501FE9BEE0F950B6C77087A8690
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,12 @@ private IUploader createTeamscaleSingleProjectUploader(Instrumentation instrumen
private DelayedTeamscaleMultiProjectUploader createTeamscaleMultiProjectUploader(
Instrumentation instrumentation) {
DelayedTeamscaleMultiProjectUploader uploader = new DelayedTeamscaleMultiProjectUploader(
(project, revision) ->
new TeamscaleUploader(teamscaleServer.withProjectAndRevision(project, revision)));
(project, commitInfo) -> {
if (commitInfo.preferCommitDescriptorOverRevision) {
return new TeamscaleUploader(teamscaleServer.withProjectAndCommit(project, commitInfo.commit));
}
return new TeamscaleUploader(teamscaleServer.withProjectAndRevision(project, commitInfo.revision));
});

if (gitPropertiesJar != null) {
logger.info(
Expand All @@ -517,43 +521,47 @@ private DelayedTeamscaleMultiProjectUploader createTeamscaleMultiProjectUploader
}

private void startGitPropertiesSearchInJarFile(DelayedUploader<ProjectAndCommit> uploader,
File gitPropertiesJar) {
File gitPropertiesJar) {
GitSingleProjectPropertiesLocator<ProjectAndCommit> locator = new GitSingleProjectPropertiesLocator<>(uploader,
GitPropertiesLocatorUtils::getProjectRevisionsFromGitProperties, this.searchGitPropertiesRecursively);
locator.searchFileForGitPropertiesAsync(gitPropertiesJar, true);
}

private void registerSingleGitPropertiesLocator(DelayedUploader<ProjectAndCommit> uploader,
Instrumentation instrumentation) {
Instrumentation instrumentation) {
GitSingleProjectPropertiesLocator<ProjectAndCommit> locator = new GitSingleProjectPropertiesLocator<>(uploader,
GitPropertiesLocatorUtils::getProjectRevisionsFromGitProperties, this.searchGitPropertiesRecursively);
instrumentation.addTransformer(new GitPropertiesLocatingTransformer(locator, getLocationIncludeFilter()));
}

private DelayedUploader<ProjectAndCommit> createDelayedSingleProjectTeamscaleUploader() {
return new DelayedUploader<>(
projectRevision -> {
if (!StringUtils.isEmpty(projectRevision.getProject()) && !teamscaleServer.project
.equals(projectRevision.getProject())) {
projectAndCommit -> {
if (!StringUtils.isEmpty(projectAndCommit.getProject()) && !teamscaleServer.project
.equals(projectAndCommit.getProject())) {
logger.warn(
"Teamscale project '{}' specified in the agent configuration is not the same as the Teamscale project '{}' specified in git.properties file(s). Proceeding to upload to the" +
" Teamscale project '{}' specified in the agent configuration.",
teamscaleServer.project, projectRevision.getProject(), teamscaleServer.project);
teamscaleServer.project, projectAndCommit.getProject(), teamscaleServer.project);
}
if (projectAndCommit.getCommitInfo().preferCommitDescriptorOverRevision) {
teamscaleServer.commit = projectAndCommit.getCommitInfo().commit;
} else {
teamscaleServer.revision = projectAndCommit.getCommitInfo().revision;
}
teamscaleServer.revision = projectRevision.getCommitInfo().revision; // TODO
return new TeamscaleUploader(teamscaleServer);
}, outputDirectory);
}

private void startMultiGitPropertiesFileSearchInJarFile(DelayedTeamscaleMultiProjectUploader uploader,
File gitPropertiesJar) {
File gitPropertiesJar) {
GitMultiProjectPropertiesLocator locator = new GitMultiProjectPropertiesLocator(uploader,
this.searchGitPropertiesRecursively);
locator.searchFileForGitPropertiesAsync(gitPropertiesJar, true);
}

private void registerMultiGitPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader,
Instrumentation instrumentation) {
Instrumentation instrumentation) {
GitMultiProjectPropertiesLocator locator = new GitMultiProjectPropertiesLocator(uploader,
this.searchGitPropertiesRecursively);
instrumentation.addTransformer(new GitPropertiesLocatingTransformer(locator, getLocationIncludeFilter()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ public static List<CommitInfo> parseGitProperties(File file, boolean isJarFile,
CommitInfo commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties(properties, entry, file,
gitPropertiesCommitTimeFormat);
result.add(commitInfo);

}

return result;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.teamscale.jacoco.agent.upload.teamscale;

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;
Expand All @@ -12,17 +13,17 @@
/** 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, String, IUploader> uploaderFactory;
private final BiFunction<String, CommitInfo, IUploader> uploaderFactory;
private final List<IUploader> teamscaleUploaders = new ArrayList<>();

public DelayedTeamscaleMultiProjectUploader(BiFunction<String, String, IUploader> uploaderFactory) {
public DelayedTeamscaleMultiProjectUploader(BiFunction<String, CommitInfo, IUploader> uploaderFactory) {
this.uploaderFactory = uploaderFactory;
}

/** Sets the project and revision detected for the Teamscale project. */
public void setTeamscaleProjectForRevision(ProjectAndCommit projectAndCommit) {
IUploader uploader = uploaderFactory.apply(projectAndCommit.getProject(),
projectAndCommit.getCommitInfo().revision); // TODO
projectAndCommit.getCommitInfo());
teamscaleUploaders.add(uploader);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ public void testRevisionAndTimestampAreBothReadIfPresent() throws InvalidGitProp
String timestamp = "2024-05-13T16:42:03+02:00";
String revision = "ab1337cd";
String epochTimestamp = "1715611323000";
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_GIT_BRANCH, branchName);
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_GIT_COMMIT_TIME, timestamp);
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_TEAMSCALE_TIMESTAMP,
branchName + ":" + timestamp);
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_GIT_COMMIT_ID, revision);
CommitInfo commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties(properties,
"myEntry", new File("myJarFile"));
Expand All @@ -150,4 +150,27 @@ public void testRevisionAndTimestampAreBothReadIfPresent() throws InvalidGitProp
assertThat(commitInfo.revision).isNotEmpty();
}

@Test
public void testPreferCommitDescriptorOverRevisionIsSetWhenTeamscaleTimestampIsPresentInGitProperties() throws InvalidGitPropertiesException {
Properties properties = new Properties();
String branchName = "myBranch";
String timestamp = "2024-05-13T16:42:03+02:00";
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_TEAMSCALE_TIMESTAMP,
branchName + ":" + timestamp);
CommitInfo commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties(properties,
"myEntry", new File("myJarFile"));
assertThat(commitInfo.preferCommitDescriptorOverRevision).isTrue();
}

@Test
public void testPreferCommitDescriptorOverRevisionIsNotSetWhenTeamscaleTimestampIsNotPresentInGitProperties() throws InvalidGitPropertiesException {
Properties properties = new Properties();
String branchName = "myBranch";
String timestamp = "2024-05-13T16:42:03+02:00";
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_GIT_BRANCH, branchName);
properties.setProperty(GitPropertiesLocatorUtils.GIT_PROPERTIES_GIT_COMMIT_TIME, timestamp);
CommitInfo commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties(properties,
"myEntry", new File("myJarFile"));
assertThat(commitInfo.preferCommitDescriptorOverRevision).isFalse();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.teamscale.jacoco.agent.upload.delay;

import com.teamscale.jacoco.agent.commit_resolution.git_properties.CommitInfo;
import com.teamscale.jacoco.agent.commit_resolution.git_properties.GitPropertiesLocatorUtils;
import com.teamscale.jacoco.agent.commit_resolution.git_properties.GitSingleProjectPropertiesLocator;
import com.teamscale.jacoco.agent.util.InMemoryUploader;
Expand Down Expand Up @@ -27,12 +28,12 @@ public void locatorShouldTriggerUploadOfCachedXmls(@TempDir Path outputPath) thr
CoverageFile coverageFile = new CoverageFile(Files.createFile(coverageFilePath).toFile());

InMemoryUploader destination = new InMemoryUploader();
DelayedUploader<String> store = new DelayedUploader<>(commit -> destination, outputPath,
DelayedUploader<CommitInfo> store = new DelayedUploader<>(commit -> destination, outputPath,
storeExecutor);

ExecutorService locatorExecutor = Executors.newSingleThreadExecutor();
GitSingleProjectPropertiesLocator<String> locator = new GitSingleProjectPropertiesLocator<>(store,
GitPropertiesLocatorUtils::getRevisionsFromGitProperties, locatorExecutor, true);
GitSingleProjectPropertiesLocator<CommitInfo> locator = new GitSingleProjectPropertiesLocator<>(store,
GitPropertiesLocatorUtils::getCommitInfoFromGitProperties, locatorExecutor, true);

store.upload(coverageFile);
locator.searchFileForGitPropertiesAsync(new File(getClass().getResource("git-properties.jar").toURI()), true);
Expand Down

0 comments on commit 2001810

Please sign in to comment.