From b5438b65fef44757dc498fc2ebfd8ae0a9d873ee Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 3 Jun 2023 12:04:47 +0200 Subject: [PATCH 1/6] [MRESOLVER-369] Introduce update check policy scope This change introduces new configuration: update check policy scope, that limits where (artifact, metadata, both) the update policy is applied. If update policy is not applied, presence/absence decides instead. To achieve "old" behaviour (ie. for use in Maven3), the configuration properties should always be set to "all" in session factory. --- https://issues.apache.org/jira/browse/MRESOLVER-369 --- .../impl/DefaultUpdateCheckManager.java | 65 +++++++++++++++++-- .../impl/DefaultUpdateCheckManagerTest.java | 25 ++++++- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java index 367bc2b2c..aaa385df5 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java @@ -23,12 +23,7 @@ import javax.inject.Singleton; import java.io.File; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Properties; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import org.eclipse.aether.RepositorySystemSession; @@ -87,6 +82,20 @@ public String toString() { private static final int STATE_DISABLED = 2; + /** + * Configuration property key for effective {@link UpdatePolicyScope}, defaults to {@link #DEFAULT_UPDATE_POLICY_SCOPE}. + * + * @since TBD + */ + static final String CONFIG_PROP_UPDATE_POLICY_SCOPE = "aether.updateCheckManager.updatePolicyScope"; + + /** + * The default value for {@link #CONFIG_PROP_UPDATE_POLICY_SCOPE}, {@link UpdatePolicyScope#METADATA}. + * + * @since TBD + */ + static final UpdatePolicyScope DEFAULT_UPDATE_POLICY_SCOPE = UpdatePolicyScope.METADATA; + /** * This "last modified" timestamp is used when no local file is present, signaling "first attempt" to cache a file, * but as it is not present, outcome is simply always "go get it". @@ -106,6 +115,17 @@ public String toString() { */ private static final long TS_UNKNOWN = 1L; + /** + * Update policy scope defines to what the policy is applied. If policy is not applied, the presence or absence + * matters only. + * + * @since TBD + */ + private enum UpdatePolicyScope { + ALL, // Maven3.x behaviour: applies to metadata and artifacts + METADATA // Applies ONLY to metadata (as artifacts are immutable) + } + public DefaultUpdateCheckManager() { // default ctor for ServiceLocator } @@ -116,6 +136,7 @@ public DefaultUpdateCheckManager() { setUpdatePolicyAnalyzer(updatePolicyAnalyzer); } + @Override public void initService(ServiceLocator locator) { setTrackingFileManager(locator.getService(TrackingFileManager.class)); setUpdatePolicyAnalyzer(locator.getService(UpdatePolicyAnalyzer.class)); @@ -131,6 +152,7 @@ public DefaultUpdateCheckManager setUpdatePolicyAnalyzer(UpdatePolicyAnalyzer up return this; } + @Override public void checkArtifact(RepositorySystemSession session, UpdateCheck check) { requireNonNull(session, "session cannot be null"); requireNonNull(check, "check cannot be null"); @@ -150,6 +172,11 @@ public void checkArtifact(RepositorySystemSession session, UpdateCheck check) { requireNonNull(session, "session cannot be null"); requireNonNull(check, "check cannot be null"); @@ -460,6 +488,7 @@ private Properties read(File touchFile) { return (props != null) ? props : new Properties(); } + @Override public void touchArtifact(RepositorySystemSession session, UpdateCheck check) { requireNonNull(session, "session cannot be null"); requireNonNull(check, "check cannot be null"); @@ -487,6 +516,7 @@ private boolean hasErrors(Properties props) { return false; } + @Override public void touchMetadata(RepositorySystemSession session, UpdateCheck check) { requireNonNull(session, "session cannot be null"); requireNonNull(check, "check cannot be null"); @@ -526,4 +556,27 @@ private Properties write(File touchFile, String dataKey, String transferKey, Exc return trackingFileManager.update(touchFile, updates); } + + /** + * Returns the configured {@link UpdatePolicyScope} or default value {@link #DEFAULT_UPDATE_POLICY_SCOPE}. + */ + private UpdatePolicyScope getUpdatePolicyScope(RepositorySystemSession session, RemoteRepository repository) { + String scope = ConfigUtils.getString( + session, + "", + CONFIG_PROP_UPDATE_POLICY_SCOPE + "." + repository.getId(), + CONFIG_PROP_UPDATE_POLICY_SCOPE); + if (scope != null && !scope.isEmpty()) { + try { + return UpdatePolicyScope.valueOf(scope.toUpperCase(Locale.ENGLISH)); + } catch (IllegalArgumentException e) { + LOGGER.warn( + "Illegal value for '{}': '{}' (allowed case-insensitive values are {})", + CONFIG_PROP_UPDATE_POLICY_SCOPE, + scope, + UpdatePolicyScope.values()); + } + } + return DEFAULT_UPDATE_POLICY_SCOPE; + } } diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java index ab61d5754..d4546c03d 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java @@ -20,9 +20,7 @@ import java.io.File; import java.net.URI; -import java.util.Calendar; -import java.util.Date; -import java.util.TimeZone; +import java.util.*; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositorySystemSession; @@ -43,15 +41,26 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; +import static org.junit.Assume.assumeThat; /** */ +@RunWith(Parameterized.class) public class DefaultUpdateCheckManagerTest { + @Parameterized.Parameters + public static Collection data() { + return Arrays.asList(new Object[][] {{"all"}, {"metadata"}}); + } private static final long HOUR = 60L * 60L * 1000L; + private final String updatePolicyScope; + private DefaultUpdateCheckManager manager; private DefaultRepositorySystemSession session; @@ -62,6 +71,10 @@ public class DefaultUpdateCheckManagerTest { private Artifact artifact; + public DefaultUpdateCheckManagerTest(String updatePolicyScope) { + this.updatePolicyScope = updatePolicyScope; + } + @Before public void setup() throws Exception { File dir = TestFileUtils.createTempFile(""); @@ -73,6 +86,7 @@ public void setup() throws Exception { TestFileUtils.writeString(artifactFile, "artifact"); session = TestUtils.newSession(); + session.setConfigProperty(DefaultUpdateCheckManager.CONFIG_PROP_UPDATE_POLICY_SCOPE, updatePolicyScope); repository = new RemoteRepository.Builder( "id", "default", @@ -445,6 +459,7 @@ public void testCheckArtifactFailOnNoFile() { @Test public void testCheckArtifactUpdatePolicyRequired() { + assumeThat(updatePolicyScope, equalTo("all")); UpdateCheck check = newArtifactCheck(); check.setItem(artifact); check.setFile(artifact.getFile()); @@ -500,6 +515,7 @@ public void testCheckArtifactUpdatePolicyNotRequired() { @Test public void testCheckArtifact() { + assumeThat(updatePolicyScope, equalTo("all")); UpdateCheck check = newArtifactCheck(); long fifteenMinutes = new Date().getTime() - (15L * 60L * 1000L); check.getFile().setLastModified(fifteenMinutes); @@ -613,6 +629,7 @@ public void testCheckArtifactErrorFromRepoCachingDisabled() { @Test public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() { + assumeThat(updatePolicyScope, equalTo("all")); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); @@ -629,6 +646,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() { @Test public void testCheckArtifactSessionStateModes() { + assumeThat(updatePolicyScope, equalTo("all")); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); manager.touchArtifact(session, check); @@ -677,6 +695,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_Inv @Test public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_DifferentRepoIdSameUrl() { + assumeThat(updatePolicyScope, equalTo("all")); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); From 2c8bc2bf708d380d777974ce0e2e5279bc379a20 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 3 Jun 2023 12:10:55 +0200 Subject: [PATCH 2/6] Add default policy (defaults to metadata) as well --- .../aether/internal/impl/DefaultUpdateCheckManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java index d4546c03d..7841ebb5a 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java @@ -54,7 +54,7 @@ public class DefaultUpdateCheckManagerTest { @Parameterized.Parameters public static Collection data() { - return Arrays.asList(new Object[][] {{"all"}, {"metadata"}}); + return Arrays.asList(new Object[][] {{""}, {"all"}, {"metadata"}}); } private static final long HOUR = 60L * 60L * 1000L; From 9151fbdf7cef79dac64cd730a5028455c763c622 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 3 Jun 2023 12:13:51 +0200 Subject: [PATCH 3/6] Add doco --- src/site/markdown/configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/site/markdown/configuration.md b/src/site/markdown/configuration.md index 4e193dfb4..bc92c7d62 100644 --- a/src/site/markdown/configuration.md +++ b/src/site/markdown/configuration.md @@ -108,6 +108,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix `aether.trustedChecksumsSource.summaryFile.basedir` | String | The basedir path for `summaryFile` trusted checksum source. If relative, resolved against local repository root, if absolute, used as is. | `".checksums"` | no `aether.trustedChecksumsSource.summaryFile.originAware` | boolean | Is trusted checksum source origin aware (factors in Repository ID into path) or not. | `true` | no `aether.updateCheckManager.sessionState` | String | Manages the session state, i.e. influences if the same download requests to artifacts/metadata will happen multiple times within the same RepositorySystemSession. If `"enabled"` will enable the session state. If `"bypass"` will enable bypassing (i.e. store all artifact ids/metadata ids which have been updates but not evaluating those). All other values lead to disabling the session state completely. | `"enabled"` | no +`aether.updateCheckManager.updatePolicyScope` | String | Defines the scope to what update policy should be applied to. Accepted values are "all" (Maven3 behaviour, artifacts and metadata both are checked for updates) and "metadata" (only metadata is checked for updates, assuming artifacts are immutable). | `"metadata"` | yes All properties which have `yes` in the column `Supports Repo ID Suffix` can be optionally configured specifically for a repository id. In that case the configuration property needs to be suffixed with a period followed by the repository id of the repository to configure, e.g. `aether.connector.http.headers.central` for repository with id `central`. From 27db7cdfc495585abb067b5df01cd657da98e4d6 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 11 Jun 2023 12:04:03 +0200 Subject: [PATCH 4/6] Refine Previous solution did not handle errors in case of -U --- .../impl/DefaultUpdateCheckManager.java | 35 +++++++++++++------ .../impl/DefaultUpdateCheckManagerTest.java | 24 +++++++------ 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java index aaa385df5..cf74d4a7a 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java @@ -121,9 +121,25 @@ public String toString() { * * @since TBD */ - private enum UpdatePolicyScope { - ALL, // Maven3.x behaviour: applies to metadata and artifacts - METADATA // Applies ONLY to metadata (as artifacts are immutable) + enum UpdatePolicyScope { + ALL(true, true), // Maven3.x behaviour: applies to metadata and artifacts + METADATA(false, true); // Applies ONLY to metadata (as artifacts are immutable) + + private final boolean checkArtifact; + private final boolean checkMetadata; + + UpdatePolicyScope(boolean checkArtifact, boolean checkMetadata) { + this.checkArtifact = checkArtifact; + this.checkMetadata = checkMetadata; + } + + public boolean isCheckArtifact() { + return checkArtifact; + } + + public boolean isCheckMetadata() { + return checkMetadata; + } } public DefaultUpdateCheckManager() { @@ -172,11 +188,6 @@ public void checkArtifact(RepositorySystemSession session, UpdateCheck data() { private static final long HOUR = 60L * 60L * 1000L; - private final String updatePolicyScope; + private final String updatePolicyScopeString; private DefaultUpdateCheckManager manager; @@ -71,8 +70,10 @@ public static Collection data() { private Artifact artifact; - public DefaultUpdateCheckManagerTest(String updatePolicyScope) { - this.updatePolicyScope = updatePolicyScope; + private DefaultUpdateCheckManager.UpdatePolicyScope updatePolicyScope; + + public DefaultUpdateCheckManagerTest(String updatePolicyScopeString) { + this.updatePolicyScopeString = updatePolicyScopeString; } @Before @@ -86,7 +87,7 @@ public void setup() throws Exception { TestFileUtils.writeString(artifactFile, "artifact"); session = TestUtils.newSession(); - session.setConfigProperty(DefaultUpdateCheckManager.CONFIG_PROP_UPDATE_POLICY_SCOPE, updatePolicyScope); + session.setConfigProperty(DefaultUpdateCheckManager.CONFIG_PROP_UPDATE_POLICY_SCOPE, updatePolicyScopeString); repository = new RemoteRepository.Builder( "id", "default", @@ -98,6 +99,7 @@ public void setup() throws Exception { metadata = new DefaultMetadata( "gid", "aid", "ver", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT, metadataFile); artifact = new DefaultArtifact("gid", "aid", "", "ext", "ver").setFile(artifactFile); + updatePolicyScope = DefaultUpdateCheckManager.getUpdatePolicyScope(session, repository); } @After @@ -459,7 +461,7 @@ public void testCheckArtifactFailOnNoFile() { @Test public void testCheckArtifactUpdatePolicyRequired() { - assumeThat(updatePolicyScope, equalTo("all")); + assumeTrue(updatePolicyScope.isCheckArtifact()); UpdateCheck check = newArtifactCheck(); check.setItem(artifact); check.setFile(artifact.getFile()); @@ -515,7 +517,7 @@ public void testCheckArtifactUpdatePolicyNotRequired() { @Test public void testCheckArtifact() { - assumeThat(updatePolicyScope, equalTo("all")); + assumeTrue(updatePolicyScope.isCheckArtifact()); UpdateCheck check = newArtifactCheck(); long fifteenMinutes = new Date().getTime() - (15L * 60L * 1000L); check.getFile().setLastModified(fifteenMinutes); @@ -629,7 +631,7 @@ public void testCheckArtifactErrorFromRepoCachingDisabled() { @Test public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() { - assumeThat(updatePolicyScope, equalTo("all")); + assumeTrue(updatePolicyScope.isCheckArtifact()); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); @@ -646,7 +648,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() { @Test public void testCheckArtifactSessionStateModes() { - assumeThat(updatePolicyScope, equalTo("all")); + assumeTrue(updatePolicyScope.isCheckArtifact()); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); manager.touchArtifact(session, check); @@ -695,7 +697,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_Inv @Test public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_DifferentRepoIdSameUrl() { - assumeThat(updatePolicyScope, equalTo("all")); + assumeTrue(updatePolicyScope.isCheckArtifact()); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); From 06f01120458ef909efd082b142aa41845adf2106 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 11 Jun 2023 12:07:42 +0200 Subject: [PATCH 5/6] Rename, policy applies --- .../impl/DefaultUpdateCheckManager.java | 24 ++++++++++--------- .../impl/DefaultUpdateCheckManagerTest.java | 10 ++++---- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java index cf74d4a7a..4ae1a851e 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java @@ -125,20 +125,20 @@ enum UpdatePolicyScope { ALL(true, true), // Maven3.x behaviour: applies to metadata and artifacts METADATA(false, true); // Applies ONLY to metadata (as artifacts are immutable) - private final boolean checkArtifact; - private final boolean checkMetadata; + private final boolean applyToArtifact; + private final boolean applyToMetadata; - UpdatePolicyScope(boolean checkArtifact, boolean checkMetadata) { - this.checkArtifact = checkArtifact; - this.checkMetadata = checkMetadata; + UpdatePolicyScope(boolean applyToArtifact, boolean applyToMetadata) { + this.applyToArtifact = applyToArtifact; + this.applyToMetadata = applyToMetadata; } - public boolean isCheckArtifact() { - return checkArtifact; + public boolean isApplyToArtifact() { + return applyToArtifact; } - public boolean isCheckMetadata() { - return checkMetadata; + public boolean isApplyToMetadata() { + return applyToMetadata; } } @@ -224,7 +224,8 @@ public void checkArtifact(RepositorySystemSession session, UpdateCheck check = newArtifactCheck(); check.setItem(artifact); check.setFile(artifact.getFile()); @@ -517,7 +517,7 @@ public void testCheckArtifactUpdatePolicyNotRequired() { @Test public void testCheckArtifact() { - assumeTrue(updatePolicyScope.isCheckArtifact()); + assumeTrue(updatePolicyScope.isApplyToArtifact()); UpdateCheck check = newArtifactCheck(); long fifteenMinutes = new Date().getTime() - (15L * 60L * 1000L); check.getFile().setLastModified(fifteenMinutes); @@ -631,7 +631,7 @@ public void testCheckArtifactErrorFromRepoCachingDisabled() { @Test public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() { - assumeTrue(updatePolicyScope.isCheckArtifact()); + assumeTrue(updatePolicyScope.isApplyToArtifact()); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); @@ -648,7 +648,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() { @Test public void testCheckArtifactSessionStateModes() { - assumeTrue(updatePolicyScope.isCheckArtifact()); + assumeTrue(updatePolicyScope.isApplyToArtifact()); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); manager.touchArtifact(session, check); @@ -697,7 +697,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_Inv @Test public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_DifferentRepoIdSameUrl() { - assumeTrue(updatePolicyScope.isCheckArtifact()); + assumeTrue(updatePolicyScope.isApplyToArtifact()); UpdateCheck check = newArtifactCheck(); check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS); From 1d31a5a9cc3c8b0c3a5827c98afa5b3724ef4e0c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 11 Jun 2023 19:17:29 +0200 Subject: [PATCH 6/6] Simplify and adjust debug log message --- .../impl/DefaultUpdateCheckManager.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java index 4ae1a851e..fa7226dda 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java @@ -214,7 +214,7 @@ public void checkArtifact(RepositorySystemSession session, UpdateCheck