[
https://issues.apache.org/jira/browse/MRESOLVER-266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17609043#comment-17609043
]
ASF GitHub Bot commented on MRESOLVER-266:
------------------------------------------
cstamas commented on code in PR #196:
URL: https://github.com/apache/maven-resolver/pull/196#discussion_r979298887
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java:
##########
@@ -44,20 +58,41 @@
public final class DefaultSyncContextFactory
implements SyncContextFactory, Service
{
- private NamedLockFactoryAdapter namedLockFactoryAdapter;
+ private static final Logger LOGGER = LoggerFactory.getLogger(
DefaultSyncContextFactory.class );
+
+ private static final String ADAPTER_KEY =
DefaultSyncContextFactory.class.getName() + ".adapter";
+
+ private static final String NAME_MAPPER_KEY =
"aether.syncContext.named.nameMapper";
+
+ private static final String DEFAULT_NAME_MAPPER = GAVNameMapper.NAME;
+
+ private static final String FACTORY_KEY =
"aether.syncContext.named.factory";
+
+ private static final String DEFAULT_FACTORY =
LocalReadWriteLockNamedLockFactory.NAME;
+
+ private Map<String, NameMapper> nameMappers;
+
+ private Map<String, NamedLockFactory> namedLockFactories;
+
+ private final CopyOnWriteArrayList<NamedLockFactoryAdapter>
createdAdapters = new CopyOnWriteArrayList<>();
/**
* Constructor used with DI, where factories are injected and selected
based on key.
*/
@Inject
- public DefaultSyncContextFactory( final NamedLockFactorySelector selector )
+ public DefaultSyncContextFactory( final Map<String, NameMapper>
nameMappers,
+ final Map<String, NamedLockFactory>
namedLockFactories )
{
- this.namedLockFactoryAdapter = new NamedLockFactoryAdapter(
- selector.getSelectedNameMapper(),
- selector.getSelectedNamedLockFactory()
- );
+ this.nameMappers = requireNonNull( nameMappers );
+ this.namedLockFactories = requireNonNull( namedLockFactories );
Review Comment:
IMHO no: in case of empty map, simply the context factory will be defunct,
and will fail later (this part is true), BUT
BUT another very important aspect of sisu is that injected component
collections (is true for both, lists and maps) are actually dynamic (!), in a
way, that once new component is being discovered, the new entry in collection
"pops up" atomically. Or in other words, injected list/map content may be
"extended" AFTER ctor was introduced, they are more like OSGi services, as in
that elements may "come and go" as components are discovered or destroyed.
This same happened is PlexusContainer as well, but there it happened in less
correct way (no atomicity), while sisu implemented it in proper way.
All in all, I think we should not check for non emptyness, as a) resolver
itself carries components (so if injected map is empty, it means some other,
probably bigger problem, corrupt jar?), b) it may be empty in point of time of
injection/ctor invocation, but later extension may be added that extends it,
and c) this may happen with manual use of ctor, but again, we should not
consider it as valid use case
> Simplify adapter creation and align configuration for it
> --------------------------------------------------------
>
> Key: MRESOLVER-266
> URL: https://issues.apache.org/jira/browse/MRESOLVER-266
> Project: Maven Resolver
> Issue Type: Task
> Components: Resolver
> Reporter: Tamás Cservenák
> Assignee: Tamás Cservenák
> Priority: Major
> Fix For: resolver-next
>
>
> Rework how named lock factory adapter is created, it is the ONLY bit reaching
> directly to Java System Properties instead to rely on session config
> properties. This makes it impossible to control from Maven for example (as it
> is "too late").
> Proposed changes:
> * adapter should be created based on session config properties, not Java
> system properties
> * adapter should be stored within session
> * adapter creation should be vastly simplified (currently we have 4 involved
> class: selectors and default sync context)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
