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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
