Skip to content

Commit

Permalink
Introduction versioning for scoring (#402)
Browse files Browse the repository at this point in the history
  • Loading branch information
alecharp authored Nov 2, 2023
1 parent fd08789 commit b5c36cc
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<ScoringComponentResult> componentsResults) {
public record ScoreResult(String key, int value, @JsonAlias("coefficient") float weight, Set<ScoringComponentResult> componentsResults, int version) {
public ScoreResult {
if (weight > 1) {
throw new IllegalArgumentException("Value and Coefficient must be less or equal to 1.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,9 @@ public float weight() {
public String description() {
return "Scores plugin based on its deprecation status.";
}

@Override
public int version() {
return 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public Function<Set<ScoringComponentResult>, 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());
};
}

Expand Down Expand Up @@ -123,4 +123,12 @@ public Set<Characteristics> 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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,6 @@ public void shouldScorePluginBasedOnProbeResultMatrix(Map<String, ProbeResult> 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()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,19 @@ public Score runOn(Plugin plugin) {
.map(ProbeResult::timestamp).max(Comparator.naturalOrder());
final Optional<Score> 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ public String description() {
public List<ScoringComponent> getComponents() {
return List.of();
}

@Override
public int version() {
return 0;
}
}
));

Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));

Expand All @@ -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);
}
}

0 comments on commit b5c36cc

Please sign in to comment.