[
https://issues.apache.org/jira/browse/MRESOLVER-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17784612#comment-17784612
]
ASF GitHub Bot commented on MRESOLVER-302:
------------------------------------------
gnodet commented on code in PR #360:
URL: https://github.com/apache/maven-resolver/pull/360#discussion_r1388076564
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -88,12 +88,18 @@ interface CloseableRepositorySystemSession extends
RepositorySystemSession, Clos
}
/**
- * Builder for building {@link CloseableRepositorySystemSession}
instances. Builder instances can be created with
+ * Builder for building {@link CloseableSession} instances. Builder
instances can be created with
* {@link RepositorySystem#createSessionBuilder()} method.
+ * <p>
+ * Note: while this interface extend {@link RepositorySystemSession}, it
should NOT be used as such, it is just
+ * a helper ability, to make possible uses like {@link
#withLocalRepository(File)} method is, where
+ * "chicken or egg" situation would appear (you need session for
not-yet-built session). This class is NOT
+ * immutable nor thread safe. It is highly recommended that upon
configuring builder is done, invoke
+ * {@link #build()} method and use resulting <em>immutable session</em>
throughout your code.
*
* @since TBD
*/
- interface SessionBuilder {
+ interface SessionBuilder extends RepositorySystemSession {
Review Comment:
I think this part can be moved to the DefaultSessionBuilder.
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -386,12 +393,48 @@ interface SessionBuilder {
SessionBuilder setCache(RepositoryCache cache);
/**
- * Shortcut method to set up local repository manager.
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} passed in this method. In case multiple files, session
builder will use chained local repository
+ * manager.
+ *
+ * @param basedir The local repository base directories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepositoryBasedir(File... basedir);
Review Comment:
rename to `setLocalRepositoryBasedirs(File... basedirs)`
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -386,12 +393,48 @@ interface SessionBuilder {
SessionBuilder setCache(RepositoryCache cache);
/**
- * Shortcut method to set up local repository manager.
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} passed in this method. In case multiple files, session
builder will use chained local repository
+ * manager.
+ *
+ * @param basedir The local repository base directories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepositoryBasedir(File... basedir);
+
+ /**
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} present in passed in list. In case multiple files,
session builder will use chained local
+ * repository manager.
*
- * @param basedir The local repository base directory, may be {@code
null} if none.
+ * @param basedir The local repository base directories.
* @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
*/
- SessionBuilder withLocalRepository(File basedir);
+ SessionBuilder withLocalRepositoryBasedir(List<File> basedir);
+
+ /**
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link LocalRepository} passed in this method. In case multiple
local repositories, session builder will
+ * use chained local repository manager.
+ *
+ * @param localRepository The local repositories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepository(LocalRepository... localRepository);
+
+ /**
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link LocalRepository} present in passed in list. In case multiple
local repositories, session builder will
+ * use chained local repository manager.
+ *
+ * @param basedir The local repository base directories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepository(List<LocalRepository> basedir);
Review Comment:
s/with/set
maybe `setLocalRepositories` ?
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -401,10 +444,32 @@ interface SessionBuilder {
*/
SessionBuilder withRepositorySystemSession(RepositorySystemSession
session);
+ /**
+ * Factory method that creates local repository manager using
configuration from this builder. The created
+ * manager may be chained, if more than one local repository is passed
in.
+ *
+ * @param localRepositories The ordered local repositories to create
manager for, must not be
+ * {@code null} nor empty, at least one
member must be present.
+ * @return The local repository manager, never {@code null}.
+ * @throws IllegalArgumentException If the specified repository type
is not recognized.
+ */
+ LocalRepositoryManager newLocalRepositoryManager(LocalRepository...
localRepositories);
Review Comment:
if chained LRM is now built-in, do we need those `newLocalRepositoryManager`
methods ?
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -386,12 +393,48 @@ interface SessionBuilder {
SessionBuilder setCache(RepositoryCache cache);
/**
- * Shortcut method to set up local repository manager.
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} passed in this method. In case multiple files, session
builder will use chained local repository
+ * manager.
+ *
+ * @param basedir The local repository base directories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepositoryBasedir(File... basedir);
+
+ /**
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} present in passed in list. In case multiple files,
session builder will use chained local
+ * repository manager.
*
- * @param basedir The local repository base directory, may be {@code
null} if none.
+ * @param basedir The local repository base directories.
* @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
*/
- SessionBuilder withLocalRepository(File basedir);
+ SessionBuilder withLocalRepositoryBasedir(List<File> basedir);
Review Comment:
s/with/set
this should be plural
`setLocalRepositoryBasedirs(List<File> basedirs)`
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -386,12 +393,48 @@ interface SessionBuilder {
SessionBuilder setCache(RepositoryCache cache);
/**
- * Shortcut method to set up local repository manager.
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} passed in this method. In case multiple files, session
builder will use chained local repository
+ * manager.
+ *
+ * @param basedir The local repository base directories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepositoryBasedir(File... basedir);
+
+ /**
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link File} present in passed in list. In case multiple files,
session builder will use chained local
+ * repository manager.
*
- * @param basedir The local repository base directory, may be {@code
null} if none.
+ * @param basedir The local repository base directories.
* @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
*/
- SessionBuilder withLocalRepository(File basedir);
+ SessionBuilder withLocalRepositoryBasedir(List<File> basedir);
+
+ /**
+ * Shortcut method to set up local repository manager directly onto
builder. There must be at least one non-null
+ * {@link LocalRepository} passed in this method. In case multiple
local repositories, session builder will
+ * use chained local repository manager.
+ *
+ * @param localRepository The local repositories.
+ * @return This session for chaining, never {@code null}.
+ * @see #newLocalRepositoryManager(LocalRepository...)
+ */
+ SessionBuilder withLocalRepository(LocalRepository... localRepository);
Review Comment:
s/with/set
maybe plural ? `setLocalReositories` (same for the parameter name)
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/session/DefaultSessionBuilder.java:
##########
@@ -48,18 +50,15 @@
import org.eclipse.aether.resolution.ArtifactDescriptorPolicy;
import org.eclipse.aether.resolution.ResolutionErrorPolicy;
import org.eclipse.aether.transfer.TransferListener;
+import org.eclipse.aether.util.repository.ChainedLocalRepositoryManager;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
/**
- * A default implementation of session builder.
- * <p>
- * Note: while this class implements {@link RepositorySystemSession}, it
should NOT be used as such, it is just
- * an internal technical detail to allow this class some more functional
helper abilities, like
- * {@link #withLocalRepository(File)} method is, where "chicken or egg"
situation would be present. This class is
- * NOT immutable nor thread safe.
+ * A default implementation of session builder. Is not immutable nor
thread-safe.
*/
-public final class DefaultSessionBuilder implements SessionBuilder,
RepositorySystemSession {
+public final class DefaultSessionBuilder implements SessionBuilder {
Review Comment:
I think we can keep the `implements RepositorySystemSession` on the instance
rather than on the builder api. It may even be removed if we move the creation
of the LRM inside the constructor of the `DefaultCloseableSession`, passing the
`List<LocalRepository>` instead of the LRM
> Introduce onSession close
> -------------------------
>
> Key: MRESOLVER-302
> URL: https://issues.apache.org/jira/browse/MRESOLVER-302
> Project: Maven Resolver
> Issue Type: Improvement
> Components: Resolver
> Reporter: Tamas Cservenak
> Assignee: Tamas Cservenak
> Priority: Major
> Fix For: 2.0.0
>
>
> The on session close callback, revisited.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
