Skip to content

[MRESOLVER-369] Introduce update check policy scope #297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cstamas wants to merge 6 commits into from
Closed

[MRESOLVER-369] Introduce update check policy scope #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5438b6
[MRESOLVER-369] Introduce update check policy scope
cstamas Jun 3, 2023
2c8bc2b
Add default policy (defaults to metadata) as well
cstamas Jun 3, 2023
9151fbd
Add doco
cstamas Jun 3, 2023
27db7cd
Refine
cstamas Jun 11, 2023
06f0112
Rename, policy applies
cstamas Jun 11, 2023
1d31a5a
Simplify and adjust debug log message
cstamas Jun 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 82 additions & 10 deletions ...solver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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".
Expand All @@ -106,6 +115,33 @@ 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
*/
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 applyToArtifact;
private final boolean applyToMetadata;

UpdatePolicyScope(boolean applyToArtifact, boolean applyToMetadata) {
this.applyToArtifact = applyToArtifact;
this.applyToMetadata = applyToMetadata;
}

public boolean isApplyToArtifact() {
return applyToArtifact;
}

public boolean isApplyToMetadata() {
return applyToMetadata;
}
}

public DefaultUpdateCheckManager() {
// default ctor for ServiceLocator
}
Expand All @@ -116,6 +152,7 @@ public DefaultUpdateCheckManager() {
setUpdatePolicyAnalyzer(updatePolicyAnalyzer);
}

@Override
public void initService(ServiceLocator locator) {
setTrackingFileManager(locator.getService(TrackingFileManager.class));
setUpdatePolicyAnalyzer(locator.getService(UpdatePolicyAnalyzer.class));
Expand All @@ -131,6 +168,7 @@ public DefaultUpdateCheckManager setUpdatePolicyAnalyzer(UpdatePolicyAnalyzer up
return this;
}

@Override
public void checkArtifact(RepositorySystemSession session, UpdateCheck<Artifact, ArtifactTransferException> check) {
requireNonNull(session, "session cannot be null");
requireNonNull(check, "check cannot be null");
Expand Down Expand Up @@ -176,6 +214,7 @@ public void checkArtifact(RepositorySystemSession session, UpdateCheck<Artifact,
lastUpdated = getLastUpdated(props, transferKey);
}

boolean policyApplies = getUpdatePolicyScope(session, repository).isApplyToArtifact();
if (lastUpdated == TS_NEVER) {
check.setRequired(true);
} else if (isAlreadyUpdated(session, updateKey)) {
Expand All @@ -185,10 +224,13 @@ public void checkArtifact(RepositorySystemSession session, UpdateCheck<Artifact,
if (error != null) {
check.setException(newException(error, artifact, repository));
}
} else if (isUpdatedRequired(session, lastUpdated, check.getPolicy())) {
} else if (policyApplies && isUpdatedRequired(session, lastUpdated, check.getPolicy())) {
check.setRequired(true);
} else if (fileExists) {
LOGGER.debug("Skipped remote request for {}, locally cached artifact up-to-date", check.getItem());
LOGGER.debug(
"Skipped remote request for {}, locally cached artifact is {}",
check.getItem(),
policyApplies ? "up-to-date" : "present");

check.setRequired(false);
} else {
Expand Down Expand Up @@ -236,6 +278,7 @@ private ArtifactTransferException newException(String error, Artifact artifact,
}
}

@Override
public void checkMetadata(RepositorySystemSession session, UpdateCheck<Metadata, MetadataTransferException> check) {
requireNonNull(session, "session cannot be null");
requireNonNull(check, "check cannot be null");
Expand Down Expand Up @@ -281,6 +324,7 @@ public void checkMetadata(RepositorySystemSession session, UpdateCheck<Metadata,
lastUpdated = getLastUpdated(props, transferKey);
}

boolean policyApplies = getUpdatePolicyScope(session, repository).isApplyToMetadata();
if (lastUpdated == TS_NEVER) {
check.setRequired(true);
} else if (isAlreadyUpdated(session, updateKey)) {
Expand All @@ -290,10 +334,13 @@ public void checkMetadata(RepositorySystemSession session, UpdateCheck<Metadata,
if (error != null) {
check.setException(newException(error, metadata, repository));
}
} else if (isUpdatedRequired(session, lastUpdated, check.getPolicy())) {
} else if (policyApplies && isUpdatedRequired(session, lastUpdated, check.getPolicy())) {
check.setRequired(true);
} else if (fileExists) {
LOGGER.debug("Skipped remote request for {}, locally cached metadata up-to-date", check.getItem());
LOGGER.debug(
"Skipped remote request for {}, locally cached metadata is {}",
check.getItem(),
policyApplies ? "up-to-date" : "present");

check.setRequired(false);
} else {
Expand Down Expand Up @@ -460,6 +507,7 @@ private Properties read(File touchFile) {
return (props != null) ? props : new Properties();
}

@Override
public void touchArtifact(RepositorySystemSession session, UpdateCheck<Artifact, ArtifactTransferException> check) {
requireNonNull(session, "session cannot be null");
requireNonNull(check, "check cannot be null");
Expand Down Expand Up @@ -487,6 +535,7 @@ private boolean hasErrors(Properties props) {
return false;
}

@Override
public void touchMetadata(RepositorySystemSession session, UpdateCheck<Metadata, MetadataTransferException> check) {
requireNonNull(session, "session cannot be null");
requireNonNull(check, "check cannot be null");
Expand Down Expand Up @@ -526,4 +575,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}.
*/
static 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;
}
}
27 changes: 24 additions & 3 deletions ...er-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,15 +41,25 @@
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.junit.Assert.*;
import static org.junit.Assume.assumeTrue;

/**
*/
@RunWith(Parameterized.class)
public class DefaultUpdateCheckManagerTest {
@Parameterized.Parameters
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {{""}, {"all"}, {"metadata"}});
}

private static final long HOUR = 60L * 60L * 1000L;

private final String updatePolicyScopeString;

private DefaultUpdateCheckManager manager;

private DefaultRepositorySystemSession session;
Expand All @@ -62,6 +70,12 @@ public class DefaultUpdateCheckManagerTest {

private Artifact artifact;

private DefaultUpdateCheckManager.UpdatePolicyScope updatePolicyScope;

public DefaultUpdateCheckManagerTest(String updatePolicyScopeString) {
this.updatePolicyScopeString = updatePolicyScopeString;
}

@Before
public void setup() throws Exception {
File dir = TestFileUtils.createTempFile("");
Expand All @@ -73,6 +87,7 @@ public void setup() throws Exception {
TestFileUtils.writeString(artifactFile, "artifact");

session = TestUtils.newSession();
session.setConfigProperty(DefaultUpdateCheckManager.CONFIG_PROP_UPDATE_POLICY_SCOPE, updatePolicyScopeString);
repository = new RemoteRepository.Builder(
"id",
"default",
Expand All @@ -84,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
Expand Down Expand Up @@ -445,6 +461,7 @@ public void testCheckArtifactFailOnNoFile() {

@Test
public void testCheckArtifactUpdatePolicyRequired() {
assumeTrue(updatePolicyScope.isApplyToArtifact());
UpdateCheck<Artifact, ArtifactTransferException> check = newArtifactCheck();
check.setItem(artifact);
check.setFile(artifact.getFile());
Expand Down Expand Up @@ -500,6 +517,7 @@ public void testCheckArtifactUpdatePolicyNotRequired() {

@Test
public void testCheckArtifact() {
assumeTrue(updatePolicyScope.isApplyToArtifact());
UpdateCheck<Artifact, ArtifactTransferException> check = newArtifactCheck();
long fifteenMinutes = new Date().getTime() - (15L * 60L * 1000L);
check.getFile().setLastModified(fifteenMinutes);
Expand Down Expand Up @@ -613,6 +631,7 @@ public void testCheckArtifactErrorFromRepoCachingDisabled() {

@Test
public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() {
assumeTrue(updatePolicyScope.isApplyToArtifact());
UpdateCheck<Artifact, ArtifactTransferException> check = newArtifactCheck();
check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS);

Expand All @@ -629,6 +648,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways() {

@Test
public void testCheckArtifactSessionStateModes() {
assumeTrue(updatePolicyScope.isApplyToArtifact());
UpdateCheck<Artifact, ArtifactTransferException> check = newArtifactCheck();
check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS);
manager.touchArtifact(session, check);
Expand Down Expand Up @@ -677,6 +697,7 @@ public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_Inv

@Test
public void testCheckArtifactAtMostOnceDuringSessionEvenIfUpdatePolicyAlways_DifferentRepoIdSameUrl() {
assumeTrue(updatePolicyScope.isApplyToArtifact());
UpdateCheck<Artifact, ArtifactTransferException> check = newArtifactCheck();
check.setPolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS);

Expand Down
1 change: 1 addition & 0 deletions src/site/markdown/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down