diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/model/ScoreResult.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/model/ScoreResult.java index 30ca6cc84..940db601f 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/model/ScoreResult.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/model/ScoreResult.java @@ -35,7 +35,7 @@ * @param weight the importance of the score facing the others * @param componentsResults a list of {@link ScoringComponentResult} which explain the score */ -public record ScoreResult(String key, int value, @JsonAlias("coefficient") float weight, Set componentsResults) { +public record ScoreResult(String key, int value, @JsonAlias("coefficient") float weight, Set componentsResults, int version) { public ScoreResult { if (weight > 1) { throw new IllegalArgumentException("Value and Coefficient must be less or equal to 1."); diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/AdoptionScoring.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/AdoptionScoring.java index 901264a28..2ef5adad5 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/AdoptionScoring.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/AdoptionScoring.java @@ -139,4 +139,9 @@ public float weight() { public String description() { return "Scores plugin based on the time between the last commit and the last release."; } + + @Override + public int version() { + return 1; + } } diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/DeprecatedPluginScoring.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/DeprecatedPluginScoring.java index c85bc8a48..123404e2f 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/DeprecatedPluginScoring.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/DeprecatedPluginScoring.java @@ -87,4 +87,9 @@ public float weight() { public String description() { return "Scores plugin based on its deprecation status."; } + + @Override + public int version() { + return 1; + } } diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoring.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoring.java index 59e458f67..c68e6c669 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoring.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoring.java @@ -198,4 +198,9 @@ public String description() { Scores plugin based on Jenkinsfile presence, documentation migration, dependabot and JEP-229 configuration. """; } + + @Override + public int version() { + return 1; + } } diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/Scoring.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/Scoring.java index e8c21cec7..86f44d561 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/Scoring.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/Scoring.java @@ -80,7 +80,7 @@ public Function, ScoreResult> finisher() { final double weight = changelogResults.stream() .flatMapToDouble(changelogResult -> DoubleStream.of(changelogResult.weight())) .sum(); - return new ScoreResult(key(), (int) Math.max(0, Math.round(sum / weight)), weight(), changelogResults); + return new ScoreResult(key(), (int) Math.max(0, Math.round(sum / weight)), weight(), changelogResults, version()); }; } @@ -123,4 +123,12 @@ public Set characteristics() { public final String name() { return getClass().getSimpleName(); } + + /** + * Returns the version of the scoring implementation. + * When a scoring implementation is changed, this needs to be updated. + * + * @return an integer representing the scoring implementation version. + */ + public abstract int version(); } diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/SecurityWarningScoring.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/SecurityWarningScoring.java index 7ea455113..b128eb174 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/SecurityWarningScoring.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/SecurityWarningScoring.java @@ -82,4 +82,9 @@ public float weight() { public String description() { return "Scores plugin based on current and active security warnings."; } + + @Override + public int version() { + return 1; + } } diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/UpdateCenterPublishedPluginDetectionScoring.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/UpdateCenterPublishedPluginDetectionScoring.java index 0f252eeae..e675ee62c 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/UpdateCenterPublishedPluginDetectionScoring.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/scores/UpdateCenterPublishedPluginDetectionScoring.java @@ -87,4 +87,9 @@ public float weight() { public String description() { return "Scores a plugin based on its presence or not in the update-center."; } + + @Override + public int version() { + return 1; + } } diff --git a/core/src/test/java/io/jenkins/pluginhealth/scoring/model/ScoreTest.java b/core/src/test/java/io/jenkins/pluginhealth/scoring/model/ScoreTest.java index a9ad5061b..1f288fc3d 100644 --- a/core/src/test/java/io/jenkins/pluginhealth/scoring/model/ScoreTest.java +++ b/core/src/test/java/io/jenkins/pluginhealth/scoring/model/ScoreTest.java @@ -43,15 +43,15 @@ void shouldBeAbleToAdjustScoreValueWithNewDetails() { assertThat(score.getValue()).isEqualTo(0); - score.addDetail(new ScoreResult("foo", 100, .4f, Set.of())); + score.addDetail(new ScoreResult("foo", 100, .4f, Set.of(), 1)); assertThat(score.getDetails().size()).isEqualTo(1); assertThat(score.getValue()).isEqualTo(100); - score.addDetail(new ScoreResult("bar", 0, .2f, Set.of())); + score.addDetail(new ScoreResult("bar", 0, .2f, Set.of(), 1)); assertThat(score.getDetails().size()).isEqualTo(2); assertThat(score.getValue()).isEqualTo(67); - score.addDetail(new ScoreResult("wiz", 100, .3f, Set.of())); + score.addDetail(new ScoreResult("wiz", 100, .3f, Set.of(), 1)); assertThat(score.getDetails().size()).isEqualTo(3); assertThat(score.getValue()).isEqualTo(78); } diff --git a/core/src/test/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoringTest.java b/core/src/test/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoringTest.java index 9bbba4c5f..894a7b052 100644 --- a/core/src/test/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoringTest.java +++ b/core/src/test/java/io/jenkins/pluginhealth/scoring/scores/PluginMaintenanceScoringTest.java @@ -253,6 +253,6 @@ public void shouldScorePluginBasedOnProbeResultMatrix(Map d .isNotNull() .usingRecursiveComparison() .comparingOnlyFields("key", "value", "weight") - .isEqualTo(new ScoreResult(KEY, value, COEFFICIENT, Set.of())); + .isEqualTo(new ScoreResult(KEY, value, COEFFICIENT, Set.of(), scoring.version())); } } diff --git a/core/src/test/java/io/jenkins/pluginhealth/scoring/service/ScoreServiceIT.java b/core/src/test/java/io/jenkins/pluginhealth/scoring/service/ScoreServiceIT.java index b33c688c2..235b3fea8 100644 --- a/core/src/test/java/io/jenkins/pluginhealth/scoring/service/ScoreServiceIT.java +++ b/core/src/test/java/io/jenkins/pluginhealth/scoring/service/ScoreServiceIT.java @@ -69,7 +69,7 @@ void shouldBeAbleToSaveScoreForPlugin() { ); final Score score = new Score(p1, ZonedDateTime.now()); - final ScoreResult result = new ScoreResult("foo", 100, 1, Set.of()); + final ScoreResult result = new ScoreResult("foo", 100, 1, Set.of(), 1); score.addDetail(result); final Score saved = scoreService.save(score); @@ -91,11 +91,11 @@ void shouldBeAbleToExtractScoreSummary() { ); final Score p1s = new Score(p1, ZonedDateTime.now()); - p1s.addDetail(new ScoreResult("foo", 100, 1, Set.of())); - p1s.addDetail(new ScoreResult("bar", 0, .5f, Set.of())); + p1s.addDetail(new ScoreResult("foo", 100, 1, Set.of(), 1)); + p1s.addDetail(new ScoreResult("bar", 0, .5f, Set.of(), 1)); final Score p2s = new Score(p2, ZonedDateTime.now()); - p2s.addDetail(new ScoreResult("foo", 0, 1, Set.of())); + p2s.addDetail(new ScoreResult("foo", 0, 1, Set.of(), 1)); Set.of(p1s, p2s).forEach(scoreService::save); assertThat(scoreRepository.count()).isEqualTo(2); @@ -123,22 +123,22 @@ void shouldOnlyRetrieveLatestScoreForPlugins() { ); final Score p1s = new Score(p1, ZonedDateTime.now()); - p1s.addDetail(new ScoreResult("foo", 1, 1, Set.of())); - p1s.addDetail(new ScoreResult("bar", 0, .5f, Set.of())); + p1s.addDetail(new ScoreResult("foo", 1, 1, Set.of(), 1)); + p1s.addDetail(new ScoreResult("bar", 0, .5f, Set.of(), 1)); final Score p2s = new Score(p2, ZonedDateTime.now()); - p2s.addDetail(new ScoreResult("foo", 0, 1, Set.of())); + p2s.addDetail(new ScoreResult("foo", 0, 1, Set.of(), 1)); final Score p1sOld = new Score(p1, ZonedDateTime.now().minusMinutes(10)); - p1sOld.addDetail(new ScoreResult("foo", 1, 1, Set.of())); - p1sOld.addDetail(new ScoreResult("bar", 0, .5f, Set.of())); + p1sOld.addDetail(new ScoreResult("foo", 1, 1, Set.of(), 1)); + p1sOld.addDetail(new ScoreResult("bar", 0, .5f, Set.of(), 1)); final Score p1sOld2 = new Score(p1, ZonedDateTime.now().minusMinutes(15)); - p1sOld2.addDetail(new ScoreResult("foo", 1, 1, Set.of())); - p1sOld2.addDetail(new ScoreResult("bar", 0, .5f, Set.of())); + p1sOld2.addDetail(new ScoreResult("foo", 1, 1, Set.of(), 1)); + p1sOld2.addDetail(new ScoreResult("bar", 0, .5f, Set.of(), 1)); final Score p2sOld = new Score(p2, ZonedDateTime.now().minusMinutes(10)); - p2sOld.addDetail(new ScoreResult("foo", 0, 1, Set.of())); + p2sOld.addDetail(new ScoreResult("foo", 0, 1, Set.of(), 1)); Set.of(p1s, p2s, p1sOld, p2sOld, p1sOld2).forEach(scoreService::save); assertThat(scoreRepository.count()).isEqualTo(5); @@ -183,31 +183,31 @@ void shouldBeAbeToRetrieveScoreStatisticsAndIgnoreOldScores() { )); final Score p1s = new Score(p1, ZonedDateTime.now()); - p1s.addDetail(new ScoreResult(s1Key, 50, .5f, Set.of())); + p1s.addDetail(new ScoreResult(s1Key, 50, .5f, Set.of(), 1)); final Score p1sOld = new Score(p1, ZonedDateTime.now().minusMinutes(10)); - p1sOld.addDetail(new ScoreResult(s1Key, 100, 1, Set.of())); + p1sOld.addDetail(new ScoreResult(s1Key, 100, 1, Set.of(), 1)); final Score p2s = new Score(p2, ZonedDateTime.now()); - p2s.addDetail(new ScoreResult(s1Key, 0, 1, Set.of())); + p2s.addDetail(new ScoreResult(s1Key, 0, 1, Set.of(), 1)); final Score p2sOld = new Score(p2, ZonedDateTime.now().minusMinutes(5)); - p2sOld.addDetail(new ScoreResult(s1Key, 90, 1, Set.of())); + p2sOld.addDetail(new ScoreResult(s1Key, 90, 1, Set.of(), 1)); final Score p3s = new Score(p3, ZonedDateTime.now()); - p3s.addDetail(new ScoreResult(s1Key, 100, 1, Set.of())); + p3s.addDetail(new ScoreResult(s1Key, 100, 1, Set.of(), 1)); final Score p4s = new Score(p4, ZonedDateTime.now()); - p4s.addDetail(new ScoreResult(s1Key, 75, 1, Set.of())); + p4s.addDetail(new ScoreResult(s1Key, 75, 1, Set.of(), 1)); final Score p5s = new Score(p5, ZonedDateTime.now()); - p5s.addDetail(new ScoreResult(s1Key, 80, 1, Set.of())); + p5s.addDetail(new ScoreResult(s1Key, 80, 1, Set.of(), 1)); final Score p6s = new Score(p6, ZonedDateTime.now()); - p6s.addDetail(new ScoreResult(s1Key, 42, 1, Set.of())); + p6s.addDetail(new ScoreResult(s1Key, 42, 1, Set.of(), 1)); final Score p7s = new Score(p7, ZonedDateTime.now()); - p7s.addDetail(new ScoreResult(s1Key, 0, 1, Set.of())); + p7s.addDetail(new ScoreResult(s1Key, 0, 1, Set.of(), 1)); Set.of(p1s, p1sOld, p2s, p2sOld, p3s, p4s, p5s, p6s, p7s).forEach(scoreService::save); assertThat(scoreRepository.count()).isEqualTo(9); diff --git a/war/src/main/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngine.java b/war/src/main/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngine.java index 8a58f8768..f684e6b03 100644 --- a/war/src/main/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngine.java +++ b/war/src/main/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngine.java @@ -76,8 +76,19 @@ public Score runOn(Plugin plugin) { .map(ProbeResult::timestamp).max(Comparator.naturalOrder()); final Optional latestScore = scoreService.latestScoreFor(plugin.getName()); if (latestScore.isPresent() && (latestProbeResult.isEmpty() || latestProbeResult.get().isBefore(latestScore.get().getComputedAt()))) { - LOGGER.debug("Previous score, computed at {} is still valid.", latestScore.get().getComputedAt()); - return latestScore.get(); + final Score score = latestScore.get(); + boolean scoringIsSame = scoringService.getScoringList().stream() + .anyMatch(scoring -> + score.getDetails().stream() + .filter(sr -> sr.key().equals(scoring.key())) + .findFirst() + .map(result -> scoring.version() == result.version()) + .orElseGet(() -> false) + ); + if (scoringIsSame) { + LOGGER.debug("Previous score, computed at {} is still valid.", score.getComputedAt()); + return score; + } } Score score = this.scoringService.getScoringList().stream() diff --git a/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreAPITest.java b/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreAPITest.java index e180b4b52..5f1ba9c0c 100644 --- a/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreAPITest.java +++ b/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreAPITest.java @@ -73,17 +73,17 @@ void shouldBeAbleToProvideScoresSummary() throws Exception { final Score scoreP1 = new Score(p1, ZonedDateTime.now()); scoreP1.addDetail(new ScoreResult("scoring-1", 100, 1, Set.of( new ScoringComponentResult(100, 1, List.of("There is no active security advisory for the plugin.")) - ))); + ), 1)); final Score scoreP2 = new Score(p2, ZonedDateTime.now()); scoreP2.addDetail(new ScoreResult("scoring-1", 100, 1, Set.of( new ScoringComponentResult(100, 1, List.of("There is no active security advisory for the plugin.")) - ))); + ), 1)); scoreP2.addDetail(new ScoreResult("scoring-2", 50, 1, Set.of( new ScoringComponentResult(0, 1, List.of("There is no Jenkinsfile detected on the plugin repository.")), new ScoringComponentResult(100, .5f, List.of("The plugin documentation was migrated to its repository.")), new ScoringComponentResult(100, .5f, List.of("The plugin is using dependabot.", "0 open pull requests from dependency update tool.")) - ))); + ), 1)); when(scoreService.getLatestScoresSummaryMap()).thenReturn(Map.of( "plugin-1", scoreP1, diff --git a/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreControllerTest.java b/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreControllerTest.java index 67cb3f92d..57cc7c5af 100644 --- a/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreControllerTest.java +++ b/war/src/test/java/io/jenkins/pluginhealth/scoring/http/ScoreControllerTest.java @@ -95,6 +95,11 @@ public String description() { public List getComponents() { return List.of(); } + + @Override + public int version() { + return 0; + } } )); @@ -124,7 +129,7 @@ void shouldDisplayScoreForSpecificPlugin() throws Exception { when(score.getPlugin()).thenReturn(plugin); when(score.getValue()).thenReturn(42L); when(score.getDetails()).thenReturn(Set.of( - new ScoreResult(scoreKey, 42, 1f, Set.of()) + new ScoreResult(scoreKey, 42, 1f, Set.of(), 1) )); when(scoreService.latestScoreFor(pluginName)).thenReturn(Optional.of(score)); diff --git a/war/src/test/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngineTest.java b/war/src/test/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngineTest.java index 1073e6483..03bcad930 100644 --- a/war/src/test/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngineTest.java +++ b/war/src/test/java/io/jenkins/pluginhealth/scoring/scores/ScoringEngineTest.java @@ -68,8 +68,8 @@ void shouldBeAbleToScoreOnePlugin() { final Scoring scoringB = mock(Scoring.class); when(plugin.getName()).thenReturn("foo-bar"); - when(scoringA.apply(plugin)).thenReturn(new ScoreResult("scoring-a", 100, .5f, Set.of())); - when(scoringB.apply(plugin)).thenReturn(new ScoreResult("scoring-b", 0, 1, Set.of())); + when(scoringA.apply(plugin)).thenReturn(new ScoreResult("scoring-a", 100, .5f, Set.of(), 1)); + when(scoringB.apply(plugin)).thenReturn(new ScoreResult("scoring-b", 0, 1, Set.of(), 1)); when(scoringService.getScoringList()).thenReturn(List.of(scoringA, scoringB)); when(scoreService.save(any(Score.class))).then(AdditionalAnswers.returnsFirstArg()); @@ -99,8 +99,8 @@ void shouldBeAbleToScoreMultiplePlugins() { when(pluginB.getName()).thenReturn("plugin-b"); when(pluginC.getName()).thenReturn("plugin-c"); - when(scoringA.apply(any(Plugin.class))).thenReturn(new ScoreResult("scoring-a", 100, 0.5f, Set.of())); - when(scoringB.apply(any(Plugin.class))).thenReturn(new ScoreResult("scoring-b", 75, .75f, Set.of())); + when(scoringA.apply(any(Plugin.class))).thenReturn(new ScoreResult("scoring-a", 100, 0.5f, Set.of(), 1)); + when(scoringB.apply(any(Plugin.class))).thenReturn(new ScoreResult("scoring-b", 75, .75f, Set.of(), 1)); when(scoringService.getScoringList()).thenReturn(List.of(scoringA, scoringB)); when(pluginService.streamAll()).thenReturn(Stream.of(pluginA, pluginB, pluginC)); @@ -132,15 +132,22 @@ void shouldOnlyScorePluginsWithNewerResultThanLatestScore() { "foo-bar", new ProbeResult("foo-bar", "", ProbeResult.Status.SUCCESS, ZonedDateTime.now().minusMinutes(15), 1) )); + final String scoringAKey = "scoring-A"; final Scoring scoringA = mock(Scoring.class); + when(scoringA.version()).thenReturn(1); + when(scoringA.key()).thenReturn(scoringAKey); final Score oldPluginAScore = mock(Score.class); when(oldPluginAScore.getComputedAt()).thenReturn(ZonedDateTime.now().minusMinutes(5)); + when(oldPluginAScore.getDetails()).thenReturn(Set.of( + new ScoreResult(scoringAKey, 100, 1, Set.of(), 1) + )); + when(scoringService.getScoringList()).thenReturn(List.of(scoringA)); when(scoreService.latestScoreFor(pluginName)).thenReturn(Optional.of(oldPluginAScore)); - final ScoringEngine ScoringEngine = new ScoringEngine(scoringService, pluginService, scoreService); - final Score score = ScoringEngine.runOn(pluginA); + final ScoringEngine scoringEngine = new ScoringEngine(scoringService, pluginService, scoreService); + final Score score = scoringEngine.runOn(pluginA); verify(scoringA, times(0)).apply(any(Plugin.class)); @@ -149,22 +156,42 @@ void shouldOnlyScorePluginsWithNewerResultThanLatestScore() { } @Test - void shouldNotScorePluginsWithLatestScoreAndEmptyDetailsMap() { - final Plugin pluginA = mock(Plugin.class); - final String pluginName = "plugin-a"; - when(pluginA.getName()).thenReturn(pluginName); - when(pluginA.getDetails()).thenReturn(Map.of()); - + void shouldReRunScoringWhenVersionChanged() { + final Plugin plugin = mock(Plugin.class); final Scoring scoringA = mock(Scoring.class); - final Score oldPluginAScore = mock(Score.class); - when(oldPluginAScore.getComputedAt()).thenReturn(ZonedDateTime.now().minusMinutes(5)); - when(scoreService.latestScoreFor(pluginName)).thenReturn(Optional.of(oldPluginAScore)); + final Score previousScore = mock(Score.class); + + when(plugin.getName()).thenReturn("foo-bar"); + when(plugin.getDetails()).thenReturn(Map.of( + "foo-bar", new ProbeResult("foo-bar", "", ProbeResult.Status.SUCCESS, ZonedDateTime.now().minusDays(1), 1) + )); + + when(scoringA.key()).thenReturn("scoring-a"); + when(scoringA.version()).thenReturn(2); + final ScoreResult expectedNewScoreResult = new ScoreResult("scoring-a", 100, .5f, Set.of(), 2); + when(scoringA.apply(plugin)).thenReturn( + expectedNewScoreResult + ); + + when(previousScore.getDetails()).thenReturn(Set.of( + new ScoreResult("scoring-a", 1, 1, Set.of(), 1) + )); + when(previousScore.getComputedAt()).thenReturn(ZonedDateTime.now().minusHours(1)); + + when(scoringService.getScoringList()).thenReturn(List.of(scoringA)); + when(scoreService.latestScoreFor("foo-bar")).thenReturn(Optional.of( + previousScore + )); + when(scoreService.save(any(Score.class))).then(AdditionalAnswers.returnsFirstArg()); final ScoringEngine ScoringEngine = new ScoringEngine(scoringService, pluginService, scoreService); - final Score score = ScoringEngine.runOn(pluginA); + final Score score = ScoringEngine.runOn(plugin); - verify(scoringA, times(0)).apply(any(Plugin.class)); - verify(scoreService, never()).save(any(Score.class)); - assertThat(score).isEqualTo(oldPluginAScore); + verify(scoringA).apply(plugin); + + assertThat(score).isNotNull(); + assertThat(score.getPlugin()).isEqualTo(plugin); + assertThat(score.getDetails()).hasSize(1).contains(expectedNewScoreResult); + assertThat(score.getValue()).isEqualTo(100); } }