Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void validateArtifact(org.eclipse.aether.artifact.Artifact artifact) {
new DefaultRepositoryKeyFunctionFactory()),
new DefaultRepositorySystemLifecycle(),
Collections.emptyMap(),
new DefaultRepositorySystemValidator(Collections.singletonList(countingValidator)));
new DefaultRepositorySystemValidator(Collections.singletonMap("counting", countingValidator)));
session = TestUtils.newSession();
}

Expand Down Expand Up @@ -162,7 +162,7 @@ void reentrantCallAllowsUninterpolatedExpressions() throws Exception {
new DefaultRepositorySystemLifecycle(),
Collections.emptyMap(),
new DefaultRepositorySystemValidator(
Collections.singletonList(EXPRESSION_REJECTING_VALIDATOR_FACTORY)));
Collections.singletonMap("expressionRejecting", EXPRESSION_REJECTING_VALIDATOR_FACTORY)));

// An outermost call with an uninterpolated expression MUST fail validation
VersionRangeRequest outerBadRequest =
Expand Down Expand Up @@ -221,7 +221,7 @@ public float getPriority() {
new DefaultRepositoryKeyFunctionFactory()),
new DefaultRepositorySystemLifecycle(),
Collections.singletonMap("test", decoratorFactory),
new DefaultRepositorySystemValidator(Collections.emptyList()));
new DefaultRepositorySystemValidator(Collections.emptyMap()));

// Outermost call: decorator should run
ArtifactDescriptorRequest outerRequest =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void init() {
new DefaultRepositoryKeyFunctionFactory()),
new DefaultRepositorySystemLifecycle(),
Collections.emptyMap(),
new DefaultRepositorySystemValidator(Collections.emptyList()));
new DefaultRepositorySystemValidator(Collections.emptyMap()));
session = TestUtils.newSession();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.eclipse.aether.internal.impl;

import java.util.Collections;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.aether.DefaultRepositoryCache;
import org.eclipse.aether.DefaultRepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.artifact.DefaultArtifact;
import org.eclipse.aether.collection.CollectRequest;
import org.eclipse.aether.graph.Dependency;
import org.eclipse.aether.internal.test.util.TestUtils;
import org.eclipse.aether.resolution.ArtifactDescriptorRequest;
import org.eclipse.aether.spi.validator.Validator;
import org.eclipse.aether.spi.validator.ValidatorFactory;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class DefaultRepositorySystemValidatorTest {
@Test
void abstain() {
DefaultRepositorySystemValidator validator = new DefaultRepositorySystemValidator(
Collections.singletonMap("abstain", session -> ValidatorFactory.NOOP));
ArtifactDescriptorRequest request =
new ArtifactDescriptorRequest(new DefaultArtifact("foo:bar:1.0"), Collections.emptyList(), "test");
assertDoesNotThrow(() -> validator.validateArtifactDescriptorRequest(TestUtils.newSession(), request));
}

@Test
void fail() {
DefaultRepositorySystemValidator validator =
new DefaultRepositorySystemValidator(Collections.singletonMap("abstain", session -> new Validator() {
@Override
public void validateArtifact(Artifact artifact) throws IllegalArgumentException {
throw new IllegalArgumentException("Artifact validation failed");
}
}));
ArtifactDescriptorRequest request =
new ArtifactDescriptorRequest(new DefaultArtifact("foo:bar:1.0"), Collections.emptyList(), "test");
assertThrows(
IllegalArgumentException.class,
() -> validator.validateArtifactDescriptorRequest(TestUtils.newSession(), request));
}

@Test
void withCaching() {
final ConcurrentHashMap<Integer, AtomicInteger> instanceCounter = new ConcurrentHashMap<>();
DefaultRepositorySystemValidator validator =
new DefaultRepositorySystemValidator(Collections.singletonMap("counter", session -> new Validator() {
@Override
public void validateArtifact(Artifact artifact) throws IllegalArgumentException {
instanceCounter
.computeIfAbsent(System.identityHashCode(this), k -> new AtomicInteger(0))
.incrementAndGet();
}
}));
DefaultRepositorySystemSession sameSession = TestUtils.newSession();
sameSession.setCache(new DefaultRepositoryCache()); // with cache
assertDoesNotThrow(() -> validator.validateArtifactDescriptorRequest(
sameSession,
new ArtifactDescriptorRequest(new DefaultArtifact("foo:bar:1.0"), Collections.emptyList(), "test")));
assertDoesNotThrow(() -> validator.validateArtifactDescriptorRequest(
sameSession,
new ArtifactDescriptorRequest(new DefaultArtifact("baz:foo:1.0"), Collections.emptyList(), "test")));
// one instance is
assertEquals(1, instanceCounter.size());
// used twice
assertEquals(
2,
instanceCounter
.get(instanceCounter.keySet().stream().iterator().next())
.get());
}

@Test
void withoutCaching() {
final ConcurrentHashMap<Integer, AtomicInteger> instanceCounter = new ConcurrentHashMap<>();
DefaultRepositorySystemValidator validator =
new DefaultRepositorySystemValidator(Collections.singletonMap("counter", session -> new Validator() {
@Override
public void validateArtifact(Artifact artifact) throws IllegalArgumentException {
instanceCounter
.computeIfAbsent(System.identityHashCode(this), k -> new AtomicInteger(0))
.incrementAndGet();
}
}));
DefaultRepositorySystemSession sameSession = TestUtils.newSession();
sameSession.setCache(null); // without cache
assertDoesNotThrow(() -> validator.validateArtifactDescriptorRequest(
sameSession,
new ArtifactDescriptorRequest(new DefaultArtifact("foo:bar:1.0"), Collections.emptyList(), "test")));
assertDoesNotThrow(() -> validator.validateArtifactDescriptorRequest(
sameSession,
new ArtifactDescriptorRequest(new DefaultArtifact("baz:foo:1.0"), Collections.emptyList(), "test")));
// two instances
assertEquals(2, instanceCounter.size());
for (AtomicInteger counter : instanceCounter.values()) {
// each used once
assertEquals(1, counter.get());
}
}

@Test
void dependencies() {
final List<Dependency> dependencies =
Collections.singletonList(new Dependency(new DefaultArtifact("foo:bar:1.0"), "compile"));
final List<Dependency> managedDependencies =
Collections.singletonList(new Dependency(new DefaultArtifact("baz:foo:1.0"), "compile"));

final AtomicInteger validateDependencies = new AtomicInteger(0);
final AtomicInteger validateManagedDependencies = new AtomicInteger(0);
DefaultRepositorySystemValidator validator = new DefaultRepositorySystemValidator(Collections.singletonMap(
"dependencies", session -> new Validator() {
@Override
public void validateDependency(Dependency dependency) throws IllegalArgumentException {
if (dependency.getArtifact().getArtifactId().equals("bar")) {
validateDependencies.incrementAndGet();
}
}

@Override
public void validateManagedDependency(Dependency managedDependency)
throws IllegalArgumentException {
if (managedDependency.getArtifact().getArtifactId().equals("foo")) {
validateManagedDependencies.incrementAndGet();
}
}
}));
assertDoesNotThrow(() -> validator.validateCollectRequest(
TestUtils.newSession(),
new CollectRequest(dependencies, managedDependencies, Collections.emptyList())));
assertEquals(1, validateDependencies.get());
assertEquals(1, validateManagedDependencies.get());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,20 @@ default void validateMetadata(Metadata metadata) throws IllegalArgumentException
*/
default void validateDependency(Dependency dependency) throws IllegalArgumentException {}

/**
* Validates managed dependency.
* <em>Important:</em> They are declarative constraints (version/scope/exclusion overrides) that only take effect
* when a matching dependency is encountered during collection. Validating them eagerly may reject valid builds
* where a BOM imports managed dependencies with uninterpolated property expressions (e.g. {@code ${osgi.version}})
* that are never actually used. If a managed dependency IS matched and its coordinates are invalid, the error
* will surface naturally during version resolution or artifact resolution.
*
* @param managedDependency The managed dependency to validate, never {@code null}.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: begin with lower case the
no periods at end

per Oracle guidelines

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree with this sentiment, this javadoc method is not the single one in this PR not following that, and in this codebase this is not the single one either (in fact, whole codebase follows this style). I'd leave this change to some time, and IMO the change should be done consistently across whole codebase.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, was wrong: lowercase stands but not the "no period at end"?

* @throws IllegalArgumentException if dependency is invalid.
* @since 2.0.22
*/
default void validateManagedDependency(Dependency managedDependency) throws IllegalArgumentException {}

/**
* Validates local repository.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,21 @@
* @since 2.0.8
*/
public interface ValidatorFactory {

/**
* The no-op (does not validate anything) instance of validator.
*
* @since 2.0.22
*/
Validator NOOP = new Validator() {};

/**
* Creates a new validator for the session.
* Creates a new validator for the session, or {@link #NOOP} to abstain from validation. Factory is consulted
* once per session (if cache present in session) and returned instances (even {@code null}s) are cached and reused
Comment thread
cstamas marked this conversation as resolved.
Outdated
* across single session.
*
* @param session The repository system session from which to configure the validator, must not be {@code null}.
* @return The validator for the session, never {@code null}.
* @return The validator to be used for the session, or {@link #NOOP} to abstain from validation, never {@code null}.
*/
Validator newInstance(RepositorySystemSession session);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
*/
package org.eclipse.aether.supplier;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
Expand Down Expand Up @@ -1131,18 +1129,18 @@ protected ModelCacheFactory createModelCacheFactory() {
return new DefaultModelCacheFactory();
}

private List<ValidatorFactory> validatorFactories;
private Map<String, ValidatorFactory> validatorFactories;

public final List<ValidatorFactory> getValidatorFactories() {
public final Map<String, ValidatorFactory> getValidatorFactories() {
checkClosed();
if (validatorFactories == null) {
validatorFactories = createValidatorFactories();
}
return validatorFactories;
}

protected List<ValidatorFactory> createValidatorFactories() {
return new ArrayList<>();
protected Map<String, ValidatorFactory> createValidatorFactories() {
return new HashMap<>();
}

private RepositorySystemValidator repositorySystemValidator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@
*/
package org.eclipse.aether.supplier;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
Expand Down Expand Up @@ -1156,18 +1154,18 @@ protected ModelCacheFactory createModelCacheFactory() {
return new DefaultModelCacheFactory();
}

private List<ValidatorFactory> validatorFactories;
private Map<String, ValidatorFactory> validatorFactories;

public final List<ValidatorFactory> getValidatorFactories() {
public final Map<String, ValidatorFactory> getValidatorFactories() {
checkClosed();
if (validatorFactories == null) {
validatorFactories = createValidatorFactories();
}
return validatorFactories;
}

protected List<ValidatorFactory> createValidatorFactories() {
return new ArrayList<>();
protected Map<String, ValidatorFactory> createValidatorFactories() {
return new HashMap<>();
}

private RepositorySystemValidator repositorySystemValidator;
Expand Down
Loading