Skip to content

Commit

Permalink
[MRESOLVER-377] Introduce metadataUpdatePolicy (#308)
Browse files Browse the repository at this point in the history
Split the policies applied in case of data and metadata, allowing greater control of their updates. Still, work "as before" in case of applications that migrate from 1.x resolver (they should receive no behavior change).

The crux of this change is really just to introduce two policies (to co-exist in parallel), but if application using resolver decides to use same policy for both, the original behavior of resolver returns and will seemingly behave as before (ie. like 1.9.x does).

Maven relies on metadata in these cases ONLY:
* resolving maven plugin prefix to maven plugin groupId (G level md)
* resolving snapshot version to timestamped version (V level md)

Resolver OTOH provides one more use case:
* "discovery" of existing versions for given GA (this use case is NOT used in Maven Core) (A level md)

Now, while Maven Core itself does NOT use 3rd use case, some plugins does, most notably the versions-maven-plugin. Today, everyone on Earth use this plugin along with `-U` Maven switch to pick up new stuff from remote repository, but this is total overkill, as it refreshes _everything_ (yes, even the immutable release artifacts!). The `-U` is simply a "must", to make Maven "refresh metadata" (as well, along with all the Artifacts) to pick up new versions on remote. Versions plugin did implement a ["hack" to overcome this limitation](mojohaus/versions@82e2450), that is still a hack, as it still applies to everything. Proper solution is to have means to express `-U but for metadata only`. This PR makes this possible on resolver side.

---

https://issues.apache.org/jira/browse/MRESOLVER-377
  • Loading branch information
cstamas authored Oct 13, 2023
1 parent 0b919cd commit aa3945e
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ public String getUpdatePolicy() {
return getSession().getUpdatePolicy();
}

@Override
public String getMetadataUpdatePolicy() {
return getSession().getMetadataUpdatePolicy();
}

@Override
public LocalRepository getLocalRepository() {
return getSession().getLocalRepository();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ public final class DefaultRepositorySystemSession implements RepositorySystemSes

private String updatePolicy;

private String metadataUpdatePolicy;

private LocalRepositoryManager localRepositoryManager;

private FileTransformerManager fileTransformerManager;
Expand Down Expand Up @@ -154,6 +156,7 @@ public DefaultRepositorySystemSession(RepositorySystemSession session) {
setArtifactDescriptorPolicy(session.getArtifactDescriptorPolicy());
setChecksumPolicy(session.getChecksumPolicy());
setUpdatePolicy(session.getUpdatePolicy());
setMetadataUpdatePolicy(session.getMetadataUpdatePolicy());
setLocalRepositoryManager(session.getLocalRepositoryManager());
setWorkspaceReader(session.getWorkspaceReader());
setRepositoryListener(session.getRepositoryListener());
Expand Down Expand Up @@ -293,6 +296,28 @@ public DefaultRepositorySystemSession setUpdatePolicy(String updatePolicy) {
return this;
}

@Override
public String getMetadataUpdatePolicy() {
return metadataUpdatePolicy;
}

/**
* Sets the global metadata update policy. If set, the global update policy overrides the update policies of the remote
* repositories being used for resolution.
*
* @param metadataUpdatePolicy The global update policy, may be {@code null}/empty to apply the per-repository policies.
* @return This session for chaining, never {@code null}.
* @see RepositoryPolicy#UPDATE_POLICY_ALWAYS
* @see RepositoryPolicy#UPDATE_POLICY_DAILY
* @see RepositoryPolicy#UPDATE_POLICY_NEVER
* @since TBD
*/
public DefaultRepositorySystemSession setMetadataUpdatePolicy(String metadataUpdatePolicy) {
verifyStateForMutation();
this.metadataUpdatePolicy = metadataUpdatePolicy;
return this;
}

@Override
public LocalRepository getLocalRepository() {
LocalRepositoryManager lrm = getLocalRepositoryManager();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public interface RepositorySystemSession {
String getChecksumPolicy();

/**
* Gets the global update policy. If set, the global update policy overrides the update policies of the remote
* Gets the global data update policy. If set, the global update policy overrides the update policies of the remote
* repositories being used for resolution.
*
* @return The global update policy or {@code null}/empty if not set and the per-repository policies apply.
Expand All @@ -104,6 +104,18 @@ public interface RepositorySystemSession {
*/
String getUpdatePolicy();

/**
* Gets the global metadata update policy. If set, the global update policy overrides the update policies of the remote
* repositories being used for resolution.
*
* @return The global update policy or {@code null}/empty if not set and the per-repository policies apply.
* @see RepositoryPolicy#UPDATE_POLICY_ALWAYS
* @see RepositoryPolicy#UPDATE_POLICY_DAILY
* @see RepositoryPolicy#UPDATE_POLICY_NEVER
* @since TBD
*/
String getMetadataUpdatePolicy();

/**
* Gets the local repository used during this session. This is a convenience method for
* {@link LocalRepositoryManager#getRepository()}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,26 +62,40 @@ public final class RepositoryPolicy {

private final String updatePolicy;

private final String metadataUpdatePolicy;

private final String checksumPolicy;

/**
* Creates a new policy with checksum warnings and daily update checks.
*/
public RepositoryPolicy() {
this(true, UPDATE_POLICY_DAILY, CHECKSUM_POLICY_WARN);
this(true, UPDATE_POLICY_DAILY, UPDATE_POLICY_DAILY, CHECKSUM_POLICY_WARN);
}

/**
* Creates a new policy with the specified settings (uses same update policy for data and metadata, retains old
* resolver behaviour).
*/
public RepositoryPolicy(boolean enabled, String updatePolicy, String checksumPolicy) {
this(enabled, updatePolicy, updatePolicy, checksumPolicy);
}

/**
* Creates a new policy with the specified settings.
*
* @param enabled A flag whether the associated repository should be accessed or not.
* @param updatePolicy The update interval after which locally cached data from the repository is considered stale
* and should be refetched, may be {@code null}.
* and should be re-fetched, may be {@code null}.
* @param metadataUpdatePolicy The update interval after which locally cached metadata from the repository is considered stale
* and should be re-fetched, may be {@code null}.
* @param checksumPolicy The way checksum verification should be handled, may be {@code null}.
* @since TBD
*/
public RepositoryPolicy(boolean enabled, String updatePolicy, String checksumPolicy) {
public RepositoryPolicy(boolean enabled, String updatePolicy, String metadataUpdatePolicy, String checksumPolicy) {
this.enabled = enabled;
this.updatePolicy = (updatePolicy != null) ? updatePolicy : "";
this.metadataUpdatePolicy = (metadataUpdatePolicy != null) ? metadataUpdatePolicy : "";
this.checksumPolicy = (checksumPolicy != null) ? checksumPolicy : "";
}

Expand All @@ -103,6 +117,16 @@ public String getUpdatePolicy() {
return updatePolicy;
}

/**
* Gets the update policy for locally cached metadata from the repository.
*
* @return The update policy, never {@code null}.
* @since TBD
*/
public String getMetadataUpdatePolicy() {
return metadataUpdatePolicy;
}

/**
* Gets the policy for checksum validation.
*
Expand All @@ -114,11 +138,10 @@ public String getChecksumPolicy() {

@Override
public String toString() {
StringBuilder buffer = new StringBuilder(256);
buffer.append("enabled=").append(isEnabled());
buffer.append(", checksums=").append(getChecksumPolicy());
buffer.append(", updates=").append(getUpdatePolicy());
return buffer.toString();
return "enabled=" + isEnabled()
+ ", checksums=" + getChecksumPolicy()
+ ", updates=" + getUpdatePolicy()
+ ", metadataUpdates=" + getMetadataUpdatePolicy();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public final class UpdateCheck<T, E extends RepositoryException> {

private String policy;

private String metadataPolicy;

private RemoteRepository repository;

private RemoteRepository authoritativeRepository;
Expand Down Expand Up @@ -147,19 +149,30 @@ public UpdateCheck<T, E> setFileValid(boolean fileValid) {
}

/**
* Gets the policy to use for the check.
* Gets the policy to use for the data check.
*
* @return The policy to use for the check.
* @return The policy to use for the data check.
* @see org.eclipse.aether.repository.RepositoryPolicy
*/
public String getPolicy() {
return policy;
}

/**
* Gets the policy to use for the metadata check.
*
* @return The policy to use for the metadata check.
* @see org.eclipse.aether.repository.RepositoryPolicy
* @since TBD
*/
public String getMetadataPolicy() {
return metadataPolicy;
}

/**
* Sets the policy to use for the check.
*
* @param policy The policy to use for the check, may be {@code null}.
* @param policy The policy to use for the data check, may be {@code null}.
* @return This object for chaining.
* @see org.eclipse.aether.repository.RepositoryPolicy
*/
Expand All @@ -168,6 +181,19 @@ public UpdateCheck<T, E> setPolicy(String policy) {
return this;
}

/**
* Sets the policy to use for the check.
*
* @param metadataPolicy The policy to use for the metadata check, may be {@code null}.
* @return This object for chaining.
* @see org.eclipse.aether.repository.RepositoryPolicy
* @since TBD
*/
public UpdateCheck<T, E> setMetadataPolicy(String metadataPolicy) {
this.metadataPolicy = metadataPolicy;
return this;
}

/**
* Gets the repository from which a potential update/download will performed.
*
Expand Down Expand Up @@ -256,6 +282,6 @@ public UpdateCheck<T, E> setException(E exception) {

@Override
public String toString() {
return getPolicy() + ": " + getFile() + " < " + getRepository();
return getPolicy() + "/" + getMetadataPolicy() + ": " + getFile() + " < " + getRepository();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ private List<ArtifactDownload> gatherDownloads(RepositorySystemSession session,
check.setFileValid(false);
check.setRepository(group.repository);
check.setPolicy(policy.getUpdatePolicy());
check.setMetadataPolicy(policy.getMetadataUpdatePolicy());
item.updateCheck = check;
updateCheckManager.checkArtifact(session, check);
if (!check.isRequired()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ private List<MetadataResult> resolve(
List<UpdateCheck<Metadata, MetadataTransferException>> checks = new ArrayList<>();
Exception exception = null;
for (RemoteRepository repo : repositories) {
RepositoryPolicy policy = getPolicy(session, repo, metadata.getNature());

UpdateCheck<Metadata, MetadataTransferException> check = new UpdateCheck<>();
check.setLocalLastUpdated((localLastUpdate != null) ? localLastUpdate : 0);
check.setItem(metadata);
Expand All @@ -294,8 +296,8 @@ private List<MetadataResult> resolve(
check.setFile(checkFile);
check.setRepository(repository);
check.setAuthoritativeRepository(repo);
check.setPolicy(
getPolicy(session, repo, metadata.getNature()).getUpdatePolicy());
check.setPolicy(policy.getUpdatePolicy());
check.setMetadataPolicy(policy.getMetadataUpdatePolicy());

if (lrmResult.isStale()) {
checks.add(check);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,25 +256,41 @@ private RepositoryPolicy merge(

if (policy2 == null) {
if (globalPolicy) {
policy = merge(policy1, session.getUpdatePolicy(), session.getChecksumPolicy());
policy = merge(
policy1,
session.getUpdatePolicy(),
session.getMetadataUpdatePolicy(),
session.getChecksumPolicy());
} else {
policy = policy1;
}
} else if (policy1 == null) {
if (globalPolicy) {
policy = merge(policy2, session.getUpdatePolicy(), session.getChecksumPolicy());
policy = merge(
policy2,
session.getUpdatePolicy(),
session.getMetadataUpdatePolicy(),
session.getChecksumPolicy());
} else {
policy = policy2;
}
} else if (!policy2.isEnabled()) {
if (globalPolicy) {
policy = merge(policy1, session.getUpdatePolicy(), session.getChecksumPolicy());
policy = merge(
policy1,
session.getUpdatePolicy(),
session.getMetadataUpdatePolicy(),
session.getChecksumPolicy());
} else {
policy = policy1;
}
} else if (!policy1.isEnabled()) {
if (globalPolicy) {
policy = merge(policy2, session.getUpdatePolicy(), session.getChecksumPolicy());
policy = merge(
policy2,
session.getUpdatePolicy(),
session.getMetadataUpdatePolicy(),
session.getChecksumPolicy());
} else {
policy = policy2;
}
Expand All @@ -296,24 +312,35 @@ private RepositoryPolicy merge(
updates = updatePolicyAnalyzer.getEffectiveUpdatePolicy(
session, policy1.getUpdatePolicy(), policy2.getUpdatePolicy());
}
String metadataUpdates = session.getMetadataUpdatePolicy();
if (globalPolicy && metadataUpdates != null && !metadataUpdates.isEmpty()) {
// use global override
} else {
metadataUpdates = updatePolicyAnalyzer.getEffectiveUpdatePolicy(
session, policy1.getMetadataUpdatePolicy(), policy2.getMetadataUpdatePolicy());
}

policy = new RepositoryPolicy(true, updates, checksums);
policy = new RepositoryPolicy(true, updates, metadataUpdates, checksums);
}

return policy;
}

private RepositoryPolicy merge(RepositoryPolicy policy, String updates, String checksums) {
private RepositoryPolicy merge(RepositoryPolicy policy, String updates, String metadataUpdates, String checksums) {
if (policy != null) {
if (updates == null || updates.isEmpty()) {
updates = policy.getUpdatePolicy();
}
if (metadataUpdates == null || metadataUpdates.isEmpty()) {
metadataUpdates = policy.getMetadataUpdatePolicy();
}
if (checksums == null || checksums.isEmpty()) {
checksums = policy.getChecksumPolicy();
}
if (!policy.getUpdatePolicy().equals(updates)
|| !policy.getMetadataUpdatePolicy().equals(metadataUpdates)
|| !policy.getChecksumPolicy().equals(checksums)) {
policy = new RepositoryPolicy(policy.isEnabled(), updates, checksums);
policy = new RepositoryPolicy(policy.isEnabled(), updates, metadataUpdates, checksums);
}
}
return policy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ public DefaultUpdateCheckManager setUpdatePolicyAnalyzer(UpdatePolicyAnalyzer up
public void checkArtifact(RepositorySystemSession session, UpdateCheck<Artifact, ArtifactTransferException> check) {
requireNonNull(session, "session cannot be null");
requireNonNull(check, "check cannot be null");
final String updatePolicy = check.getPolicy();
if (check.getLocalLastUpdated() != 0
&& !isUpdatedRequired(session, check.getLocalLastUpdated(), check.getPolicy())) {
&& !isUpdatedRequired(session, check.getLocalLastUpdated(), updatePolicy)) {
LOGGER.debug("Skipped remote request for {}, locally installed artifact up-to-date", check.getItem());

check.setRequired(false);
Expand Down Expand Up @@ -187,7 +188,7 @@ 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 (isUpdatedRequired(session, lastUpdated, updatePolicy)) {
check.setRequired(true);
} else if (fileExists) {
LOGGER.debug("Skipped remote request for {}, locally cached artifact up-to-date", check.getItem());
Expand Down Expand Up @@ -241,8 +242,9 @@ private ArtifactTransferException newException(String error, Artifact artifact,
public void checkMetadata(RepositorySystemSession session, UpdateCheck<Metadata, MetadataTransferException> check) {
requireNonNull(session, "session cannot be null");
requireNonNull(check, "check cannot be null");
final String updatePolicy = check.getMetadataPolicy();
if (check.getLocalLastUpdated() != 0
&& !isUpdatedRequired(session, check.getLocalLastUpdated(), check.getPolicy())) {
&& !isUpdatedRequired(session, check.getLocalLastUpdated(), updatePolicy)) {
LOGGER.debug("Skipped remote request for {} locally installed metadata up-to-date", check.getItem());

check.setRequired(false);
Expand Down Expand Up @@ -292,7 +294,7 @@ 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 (isUpdatedRequired(session, lastUpdated, updatePolicy)) {
check.setRequired(true);
} else if (fileExists) {
LOGGER.debug("Skipped remote request for {}, locally cached metadata up-to-date", check.getItem());
Expand Down
Loading

0 comments on commit aa3945e

Please sign in to comment.