From 110821ec29848f332f8e0f561e748acddd32dfb8 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Sat, 7 Jan 2023 22:20:26 +0100 Subject: [PATCH 1/3] Improve selectivity in Maven plugin fingerprint `PluginFingerprint` captures a particular configuration of the Spotless Maven plugin. It is used to invalidate the up-to-date index when plugin's configuration changes. Invalidations of the index file seem to be too frequent because `PluginFingerprint` includes the effective (as in Effective POM) configuration of the Spotless plugin. For example, it includes dependencies declared outside of the `` XML element. So addition of an unrelated project dependency will invalidate the index file. Spotless Maven plugin manages its own dependencies using `ArtifactResolver`. There's no need to include the effective `` object into `PluginFingerprint`. This commit simplifies the fingerprint to include only the plugin's version and serialized configuration of all formatters. It should be backward-compatible because fingerprint format change will simply trigger an invalidation of the index file. --- .../maven/incremental/PluginFingerprint.java | 21 ++- .../incremental/PluginFingerprintTest.java | 137 ++---------------- 2 files changed, 23 insertions(+), 135 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java index 3805cf4600..9efd6f2586 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2022 DiffPlug + * Copyright 2021-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,17 +17,21 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.ArrayList; import java.util.Base64; -import java.util.List; import java.util.Objects; -import org.apache.maven.model.Dependency; import org.apache.maven.model.Plugin; import org.apache.maven.project.MavenProject; import com.diffplug.spotless.Formatter; +/** + * Represents a particular Spotless Maven plugin setup using a Base64-encoded serialized form of: + *
    + *
  1. Plugin version as configured in the POM
  2. + *
  3. Formatter instances created according to the POM configuration
  4. + *
+ */ class PluginFingerprint { private static final String SPOTLESS_PLUGIN_KEY = "com.diffplug.spotless:spotless-maven-plugin"; @@ -83,12 +87,8 @@ public String toString() { } private static byte[] digest(Plugin plugin, Iterable formatters) { - // dependencies can be an unserializable org.apache.maven.model.merge.ModelMerger$MergingList - // replace it with a serializable ArrayList - List dependencies = plugin.getDependencies(); - plugin.setDependencies(new ArrayList<>(dependencies)); try (ObjectDigestOutputStream out = ObjectDigestOutputStream.create()) { - out.writeObject(plugin); + out.writeObject(plugin.getVersion()); for (Formatter formatter : formatters) { out.writeObject(formatter); } @@ -96,9 +96,6 @@ private static byte[] digest(Plugin plugin, Iterable formatters) { return out.digest(); } catch (IOException e) { throw new UncheckedIOException("Unable to serialize plugin " + plugin, e); - } finally { - // reset the original list - plugin.setDependencies(dependencies); } } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java index 90e59d657e..c344580d37 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2022 DiffPlug + * Copyright 2021-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,126 +43,22 @@ class PluginFingerprintTest extends MavenIntegrationHarness { private static final String VERSION_1 = "1.0.0"; private static final String VERSION_2 = "2.0.0"; - private static final String[] EXECUTION_1 = { - "", - " check", - " ", - " check", - " ", - "" - }; - private static final String[] EXECUTION_2 = {}; - - private static final String[] CONFIGURATION_1 = { - "", - " 1.2", - "" - }; - private static final String[] CONFIGURATION_2 = { - "", - " 1.8", - " true", - "" - }; - - private static final String[] DEPENDENCIES_1 = { - "", - " ", - " unknown", - " unknown", - " 1.0", - " ", - "" - }; - private static final String[] DEPENDENCIES_2 = { - "", - " ", - " unknown", - " unknown", - " 2.0", - " ", - "" - }; - private static final List FORMATTERS = singletonList(formatter(formatterStep("default"))); @Test - void sameFingerprint() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); - String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + void sameFingerprintWhenVersionAndFormattersAreTheSame() throws Exception { + MavenProject project = mavenProject(VERSION_1); - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); - - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); + PluginFingerprint fingerprint1 = PluginFingerprint.from(project, FORMATTERS); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project, FORMATTERS); assertThat(fingerprint1).isEqualTo(fingerprint2); } @Test - void sameFingerprintWithDependencies() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1, DEPENDENCIES_1); - String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1, DEPENDENCIES_1); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); - - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - - assertThat(fingerprint1).isEqualTo(fingerprint2); - } - - @Test - void differentFingerprintForDifferentDependencies() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1, DEPENDENCIES_1); - String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1, DEPENDENCIES_2); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); - - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - - assertThat(fingerprint1).isNotEqualTo(fingerprint2); - } - - @Test - void differentFingerprintForDifferentPluginVersion() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); - String xml2 = createPomXmlContent(VERSION_2, EXECUTION_1, CONFIGURATION_1); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); - - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - - assertThat(fingerprint1).isNotEqualTo(fingerprint2); - } - - @Test - void differentFingerprintForDifferentExecution() throws Exception { - String xml1 = createPomXmlContent(VERSION_2, EXECUTION_1, CONFIGURATION_1); - String xml2 = createPomXmlContent(VERSION_2, EXECUTION_2, CONFIGURATION_1); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); - - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - - assertThat(fingerprint1).isNotEqualTo(fingerprint2); - } - - @Test - void differentFingerprintForDifferentConfiguration() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_2, CONFIGURATION_2); - String xml2 = createPomXmlContent(VERSION_1, EXECUTION_2, CONFIGURATION_1); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); + void differentFingerprintForDifferentPluginVersions() throws Exception { + MavenProject project1 = mavenProject(VERSION_1); + MavenProject project2 = mavenProject(VERSION_2); PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); @@ -172,11 +68,8 @@ void differentFingerprintForDifferentConfiguration() throws Exception { @Test void differentFingerprintForFormattersWithDifferentSteps() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); - String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); + MavenProject project1 = mavenProject(VERSION_1); + MavenProject project2 = mavenProject(VERSION_1); FormatterStep step1 = formatterStep("step1"); FormatterStep step2 = formatterStep("step2"); @@ -192,11 +85,8 @@ void differentFingerprintForFormattersWithDifferentSteps() throws Exception { @Test void differentFingerprintForFormattersWithDifferentLineEndings() throws Exception { - String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); - String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); - - MavenProject project1 = mavenProject(xml1); - MavenProject project2 = mavenProject(xml2); + MavenProject project1 = mavenProject(VERSION_1); + MavenProject project2 = mavenProject(VERSION_1); FormatterStep step = formatterStep("step"); List formatters1 = singletonList(formatter(LineEnding.UNIX, step)); @@ -224,7 +114,8 @@ void failsWhenProjectDoesNotContainSpotlessPlugin() { .hasMessageContaining("Spotless plugin absent from the project"); } - private static MavenProject mavenProject(String xml) throws Exception { + private MavenProject mavenProject(String spotlessVersion) throws Exception { + String xml = createPomXmlContent(spotlessVersion, new String[0], new String[0]); return new MavenProject(readPom(xml)); } From 3b3d985918fb3391aea1e9593a092f1cc0303401 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Sat, 7 Jan 2023 22:21:16 +0100 Subject: [PATCH 2/3] Improve index file invalidation message in Maven plugin Do not mention plugin fingerprint because it is an implementation detail and can be confusing to users. Explain why index file invalidation could happen. --- .../com/diffplug/spotless/maven/incremental/FileIndex.java | 4 ++-- .../diffplug/spotless/maven/incremental/FileIndexTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index 728f8a57bc..8330319cfb 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 DiffPlug + * Copyright 2021-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -76,7 +76,7 @@ static FileIndex read(FileIndexConfig config, Log log) { PluginFingerprint computedFingerprint = config.getPluginFingerprint(); PluginFingerprint storedFingerprint = PluginFingerprint.from(firstLine); if (!computedFingerprint.equals(storedFingerprint)) { - log.info("Fingerprint mismatch in the index file. Fallback to an empty index"); + log.info("Index file corresponds to a different configuration of the plugin. Either the plugin version or its configuration has changed. Fallback to an empty index"); return emptyIndexFallback(config); } else { Content content = readIndexContent(reader, config.getProjectDir(), log); diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java index ce9c0077e8..8cd5e8a2f7 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 DiffPlug + * Copyright 2021-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -60,7 +60,7 @@ void readFallsBackToEmptyIndexOnFingerprintMismatch() throws Exception { FileIndex index = FileIndex.read(config, log); assertThat(index.size()).isZero(); - verify(log).info("Fingerprint mismatch in the index file. Fallback to an empty index"); + verify(log).info("Index file corresponds to a different configuration of the plugin. Either the plugin version or its configuration has changed. Fallback to an empty index"); } @Test From 016eecf893f072fbef5592e80d52cc1e0eefc8de Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Sun, 8 Jan 2023 13:59:53 +0100 Subject: [PATCH 3/3] Update changelog --- plugin-maven/CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 84e06aa847..b92aa4095d 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Fixed +* Reduce spurious invalidations of the up-to-date index file ([#1461](https://github.com/diffplug/spotless/pull/1461)) ## [2.29.0] - 2023-01-02 ### Added