From a13796e5edfd413c3de74ba372e89bdd19295047 Mon Sep 17 00:00:00 2001 From: Mark Raynsford Date: Sun, 14 Apr 2024 14:33:01 +0000 Subject: [PATCH] Restructure repository service Adjust how branches and tags are calculated, and improve error logging. Affects: https://github.com/io7m/northpike/issues/28 --- .../jgit/internal/NPSCMJGRepository.java | 83 ++++++++++++---- .../src/main/java/module-info.java | 1 + .../src/main/resources/logback.xml | 14 ++- .../server/internal/agents/NPAgentTask.java | 4 +- .../NPRepositoryArchiveCreated.java | 4 +- .../repositories/NPRepositoryCloseFailed.java | 2 +- .../repositories/NPRepositoryClosed.java | 2 +- .../NPRepositoryConfigureFailed.java | 2 +- .../repositories/NPRepositoryConfigured.java | 2 +- .../repositories/NPRepositoryService.java | 2 +- .../NPRepositoryServiceStarted.java | 2 +- .../NPRepositoryUpdateFailed.java | 2 +- .../repositories/NPRepositoryUpdated.java | 2 +- .../schedule/NPSchedulingService.java | 2 +- .../NPSCMRepositoriesJGitTest.java | 97 ++++++++++++++++--- 15 files changed, 170 insertions(+), 51 deletions(-) diff --git a/com.io7m.northpike.scm_repository.jgit/src/main/java/com/io7m/northpike/repository/jgit/internal/NPSCMJGRepository.java b/com.io7m.northpike.scm_repository.jgit/src/main/java/com/io7m/northpike/repository/jgit/internal/NPSCMJGRepository.java index 02bbcbf5..e2a49c28 100644 --- a/com.io7m.northpike.scm_repository.jgit/src/main/java/com/io7m/northpike/repository/jgit/internal/NPSCMJGRepository.java +++ b/com.io7m.northpike.scm_repository.jgit/src/main/java/com/io7m/northpike/repository/jgit/internal/NPSCMJGRepository.java @@ -49,6 +49,7 @@ import io.opentelemetry.api.trace.Span; import org.eclipse.jgit.api.ArchiveCommand; import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.ListBranchCommand; import org.eclipse.jgit.api.VerifySignatureCommand; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.archive.TgzFormat; @@ -62,6 +63,8 @@ import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.TagOpt; import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.OutputStream; @@ -108,6 +111,9 @@ public final class NPSCMJGRepository implements NPSCMRepositoryType { + private static final Logger LOG = + LoggerFactory.getLogger(NPSCMJGRepository.class); + private static final OpenOption[] TEMPORARY_FILE_OPTIONS = {TRUNCATE_EXISTING, CREATE, WRITE}; @@ -346,6 +352,7 @@ public NPArchive commitArchive( ); } catch (final Exception e) { recordSpanException(e); + LOG.error("commitArchive: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; @@ -435,6 +442,7 @@ private NPSCMCommitSet executeCalculateSince( ); } catch (final Exception e) { recordSpanException(e); + LOG.error("executeCalculateSince: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; @@ -473,14 +481,20 @@ private NPCommit transformCommit( final var commitObjectId = ObjectId.fromString(source.name()); + // final var branches = + // Set.copyOf( + // this.git.nameRev() + // .addPrefix("refs/heads") + // .add(commitObjectId) + // .call() + // .values() + // .stream() + // .map(s -> s.replace("refs/heads/", "")) + // .collect(Collectors.toSet()) + // ); + final var branches = - Set.copyOf( - this.git.nameRev() - .addPrefix("refs/heads") - .add(commitObjectId) - .call() - .values() - ); + transformGetBranchesForCommit(source); final var timeCreated = OffsetDateTime.ofInstant( @@ -495,15 +509,8 @@ private NPCommit transformCommit( final var timeReceived = OffsetDateTime.now(UTC); - final var tagList = - this.git.tagList() - .setContains(commitObjectId) - .call(); - final var tags = - tagList.stream() - .map(Ref::getName) - .collect(Collectors.toUnmodifiableSet()); + this.transformGetTagsForCommit(source); final var author = new NPCommitAuthor( @@ -526,6 +533,39 @@ private NPCommit transformCommit( ); } + private Set transformGetBranchesForCommit( + final RevCommit source) + throws GitAPIException + { + return this.git.branchList() + .setContains(source.name()) + .setListMode(ListBranchCommand.ListMode.ALL) + .call() + .stream() + .map(Ref::getName) + .map(s -> s.replace("refs/heads/", "")) + .collect(Collectors.toSet()); + } + + private Set transformGetTagsForCommit( + final RevCommit source) + throws GitAPIException + { + final var tagList = + this.git.tagList() + .call(); + + final var tagNames = new HashSet(); + for (final var ref : tagList) { + final var peeledRef = ref.getPeeledObjectId(); + if (Objects.equals(peeledRef, source.toObjectId())) { + tagNames.add(ref.getName().replace("refs/tags/", "")); + } + } + + return Set.copyOf(tagNames); + } + private void executeUpdate() throws NPSCMRepositoryException { @@ -560,6 +600,7 @@ private void executeUpdateGC() task.onCompleted(); } catch (final Exception e) { recordSpanException(e); + LOG.error("executeUpdateGC: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; @@ -589,6 +630,7 @@ private void executeUpdatePull() task.onCompleted(); } catch (final Exception e) { recordSpanException(e); + LOG.error("executeUpdatePull: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; @@ -643,19 +685,20 @@ private void executeClone() Git.cloneRepository() .setBare(true) .setCloneAllBranches(true) - .setTagOption(TagOpt.FETCH_TAGS) + .setCloneSubmodules(true) + .setCredentialsProvider(this.createCredentialsProvider()) .setGitDir(this.repoPath.toFile()) .setMirror(true) - .setURI(this.repositoryDescription.url().normalize().toString()) - .setCredentialsProvider(this.createCredentialsProvider()) .setProgressMonitor(task) - .setCloneSubmodules(true) + .setTagOption(TagOpt.FETCH_TAGS) + .setURI(this.repositoryDescription.url().normalize().toString()) .call() ); task.onCompleted(); } catch (final Exception e) { recordSpanException(e); + LOG.error("executeClone: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; @@ -756,6 +799,7 @@ public NPFingerprint commitVerifySignature( return verifier.keyUsed().fingerprint(); } catch (final Exception e) { recordSpanException(e); + LOG.error("commitVerifySignature: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; @@ -811,6 +855,7 @@ private NPSCMCommitSet executeCalculateSinceTime( ); } catch (final Exception e) { recordSpanException(e); + LOG.error("executeCalculateSinceTime: ", e); final var ex = this.handleException(e); task.onFailed(ex); throw ex; diff --git a/com.io7m.northpike.scm_repository.jgit/src/main/java/module-info.java b/com.io7m.northpike.scm_repository.jgit/src/main/java/module-info.java index 645afe8e..73c96592 100644 --- a/com.io7m.northpike.scm_repository.jgit/src/main/java/module-info.java +++ b/com.io7m.northpike.scm_repository.jgit/src/main/java/module-info.java @@ -33,6 +33,7 @@ requires com.io7m.northpike.keys; requires com.io7m.northpike.telemetry.api; + requires com.googlecode.javaewah; requires com.io7m.jmulticlose.core; requires com.io7m.lanark.core; requires io.opentelemetry.api; diff --git a/com.io7m.northpike.server.main/src/main/resources/logback.xml b/com.io7m.northpike.server.main/src/main/resources/logback.xml index 329d8f97..ad0f7916 100644 --- a/com.io7m.northpike.server.main/src/main/resources/logback.xml +++ b/com.io7m.northpike.server.main/src/main/resources/logback.xml @@ -10,14 +10,6 @@ System.err - - - - - + + + + diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/agents/NPAgentTask.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/agents/NPAgentTask.java index b8f261a2..5c9a910c 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/agents/NPAgentTask.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/agents/NPAgentTask.java @@ -311,7 +311,7 @@ private void processCommandWorkItemSent( private void processCommandLatencyCheck() throws InterruptedException, NPException, IOException { - LOG.debug("Latency: Performing check."); + LOG.trace("Latency: Performing check."); final var command = new NPACommandSLatencyCheck( @@ -328,7 +328,7 @@ private void processCommandLatencyCheck() final var duration = Duration.between(command.timeCurrent(), timeNow); - LOG.debug("Latency: RTT {}", duration); + LOG.trace("Latency: RTT {}", duration); this.metrics.setAgentLatency(this.agentId, duration); } } diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryArchiveCreated.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryArchiveCreated.java index ec5b1bea..5b18d9d5 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryArchiveCreated.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryArchiveCreated.java @@ -53,13 +53,13 @@ public NPEventSeverity severity() @Override public String name() { - return "ArchiveCreated"; + return "RepositoryArchiveCreated"; } @Override public String message() { - return "Created"; + return "RepositoryArchiveCreated"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryCloseFailed.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryCloseFailed.java index aa8edc41..0ad63cd7 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryCloseFailed.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryCloseFailed.java @@ -60,7 +60,7 @@ public String name() @Override public String message() { - return "CloseFailed"; + return "RepositoryCloseFailed"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryClosed.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryClosed.java index 70900502..e11d347d 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryClosed.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryClosed.java @@ -56,7 +56,7 @@ public String name() @Override public String message() { - return "Closed"; + return "RepositoryClosed"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigureFailed.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigureFailed.java index 1ea25781..eb8e9d20 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigureFailed.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigureFailed.java @@ -60,7 +60,7 @@ public String name() @Override public String message() { - return "ConfigureFailed"; + return "RepositoryConfigureFailed"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigured.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigured.java index f7a71ae4..57aa5d43 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigured.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryConfigured.java @@ -56,7 +56,7 @@ public String name() @Override public String message() { - return "Configured"; + return "RepositoryConfigured"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryService.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryService.java index 807c6cd3..210b2f9d 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryService.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryService.java @@ -457,7 +457,7 @@ private void processCommandTimed( { final var span = this.telemetry.tracer() - .spanBuilder("Command") + .spanBuilder("RepositoryCommand") .setAttribute("Command", command.getClass().getSimpleName()) .startSpan(); diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryServiceStarted.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryServiceStarted.java index 90b78295..f890f6d9 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryServiceStarted.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryServiceStarted.java @@ -45,7 +45,7 @@ public String name() @Override public String message() { - return "Started"; + return "RepositoryServiceStarted"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdateFailed.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdateFailed.java index 3fca370f..1ebba5ed 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdateFailed.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdateFailed.java @@ -60,7 +60,7 @@ public String name() @Override public String message() { - return "UpdateFailed"; + return "RepositoryUpdateFailed"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdated.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdated.java index e1f22a37..96c6ce48 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdated.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/repositories/NPRepositoryUpdated.java @@ -58,7 +58,7 @@ public String name() @Override public String message() { - return "Updated"; + return "RepositoryUpdated"; } @Override diff --git a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/schedule/NPSchedulingService.java b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/schedule/NPSchedulingService.java index 62349313..b0222a5a 100644 --- a/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/schedule/NPSchedulingService.java +++ b/com.io7m.northpike.server/src/main/java/com/io7m/northpike/server/internal/schedule/NPSchedulingService.java @@ -157,7 +157,7 @@ private void runScheduleTask() private void runSchedule() { - LOG.debug("Scheduling task starting"); + LOG.trace("Scheduling task starting"); final var span = this.telemetry.tracer() diff --git a/com.io7m.northpike.tests/src/main/java/com/io7m/northpike/tests/scm_repository/NPSCMRepositoriesJGitTest.java b/com.io7m.northpike.tests/src/main/java/com/io7m/northpike/tests/scm_repository/NPSCMRepositoriesJGitTest.java index 7b028187..c2fc72e9 100644 --- a/com.io7m.northpike.tests/src/main/java/com/io7m/northpike/tests/scm_repository/NPSCMRepositoriesJGitTest.java +++ b/com.io7m.northpike.tests/src/main/java/com/io7m/northpike/tests/scm_repository/NPSCMRepositoriesJGitTest.java @@ -189,6 +189,81 @@ public void testCommitsSinceEmptySet( .map(NPCommitUnqualifiedID::value) .collect(Collectors.toUnmodifiableSet()) ); + + for (final var commit : commits) { + final var commitId = commit.id().commitId().value(); + + switch (commitId) { + case "30dfe62404b3f4338297432d95e17b0630d0960c" -> { + assertEquals( + Set.of("com.io7m.example-0.0.2"), + commit.tags() + ); + } + case "b155d186fce6d0525b8348cc48dd778fda6c6a85" -> { + assertEquals( + Set.of("com.io7m.example-0.0.1"), + commit.tags() + ); + } + default -> { + assertEquals( + Set.of(), + commit.tags() + ); + } + } + } + + for (final var commit : commits) { + final var commitId = commit.id().commitId().value(); + + switch (commitId) { + case "f6d27f77259522f10f7062efca687978531456a4" -> { + final var branches = Set.of("develop"); + assertEquals( + branches, + commit.branches(), + "Commit %s must have branches %s".formatted(commitId, branches) + ); + } + case "x" -> { + final var branches = Set.of("master"); + assertEquals( + branches, + commit.branches(), + "Commit %s must have branches %s".formatted(commitId, branches) + ); + } + case "b155d186fce6d0525b8348cc48dd778fda6c6a85", + "27b00e3adce185f2b00096894e0b6bf34e9a157f", + "293d2389c729ff9574f55fcb211d4594f14193f2", + "11e4ee346c9a8708688acc4f32beac8955714b6c", + "f512486ea90cab4a8f00bc01f2ba2083f65aa0ab", + "9141a3dc3b8f0444b61aae7d4f26afcd9b6d7dbd", + "a6f4b51d85271d34e6cd79424416afaf0554d826", + "30dfe62404b3f4338297432d95e17b0630d0960c", + "5634fee48558be958830b324dd283bbde568aeb4", + "6cf9c3deec2e9663a204a8ca2c30717ff4366e5d", + "db164a1037656bfe0f4253bf9ea882ab28696b77", + "e4b4a304374886824d94b8cdf49d44c76081eeaf" -> { + final var branches = Set.of("develop", "master"); + assertEquals( + branches, + commit.branches(), + "Commit %s must have branches %s".formatted(commitId, branches) + ); + } + default -> { + final var branches = Set.of(); + assertEquals( + branches, + commit.branches(), + "Commit %s must have branches %s".formatted(commitId, branches) + ); + } + } + } } } @@ -608,17 +683,17 @@ public void testCommitUnsigned( OffsetDateTime.now(), Optional.empty(), """ ------BEGIN PGP PUBLIC KEY BLOCK----- - -mDMEZQ4TaBYJKwYBBAHaRw8BAQdAMNgv7poctgUe5VlVqPF832sXIZdqV139e/IP -ds8SwFa0QU5vcnRocGlrZSAoTm9ydGhwaWtlIEV4YW1wbGUgS2V5IEFsdGVybmF0 -aXZlKSA8bm9vbmVAZXhhbXBsZS5jb20+iJAEExYIADgWIQQPmqLJNwNrpjTRhr0T -vixRSDmHRQUCZQ4TaAIbAwULCQgHAwUVCgkICwUWAgMBAAIeAQIXgAAKCRATvixR -SDmHRZGoAQChIyLkA4TIQ6ROdJWrCgSYj0r63zu3y47wqI5U8dXjiAD/WQ3E+FA7 -WDQ4AgI9+C5Ommu8ftVPDj7dctHMEl5xEAM= -=zTld ------END PGP PUBLIC KEY BLOCK----- - """ + -----BEGIN PGP PUBLIC KEY BLOCK----- + + mDMEZQ4TaBYJKwYBBAHaRw8BAQdAMNgv7poctgUe5VlVqPF832sXIZdqV139e/IP + ds8SwFa0QU5vcnRocGlrZSAoTm9ydGhwaWtlIEV4YW1wbGUgS2V5IEFsdGVybmF0 + aXZlKSA8bm9vbmVAZXhhbXBsZS5jb20+iJAEExYIADgWIQQPmqLJNwNrpjTRhr0T + vixRSDmHRQUCZQ4TaAIbAwULCQgHAwUVCgkICwUWAgMBAAIeAQIXgAAKCRATvixR + SDmHRZGoAQChIyLkA4TIQ6ROdJWrCgSYj0r63zu3y47wqI5U8dXjiAD/WQ3E+FA7 + WDQ4AgI9+C5Ommu8ftVPDj7dctHMEl5xEAM= + =zTld + -----END PGP PUBLIC KEY BLOCK----- + """ ); private static void expandArchive(