You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/07/05 06:38:28 UTC
[GitHub] [netbeans] timboudreau opened a new pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
timboudreau opened a new pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232
One of the most common uses of `ProxyLookup` is to dynamically change the set of `Lookup`s being proxied (a quick grep finds 53 such cases in the NetBeans source base, and I can think of > 10 I have written in external modules). For each consumer of the API that needs to do this, it is necessary to write identical subclasses with a public or package visibility method that exposes one of `ProxyLookup`'s `setLookups()` methods.
This patch adds `ProxyLookup.create(Consumer<Consumer<Lookup[]>>)` and `ProxyLookup.createThreaded(Consumer<BiConsumer<Executor, Lookup[]>>) `to satisfy the non-threaded and threaded-event-delivery variants supported as protected methods on ProxyLookup. These allow a mechanism for updating the set of proxied lookups to be passed to the caller without exposing that machanism to consumers of the returned Lookup or any code other that which immediately instantiates it, which was the objection @jtulach had many years ago to exposing it as public.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-656799249
It's possible to get it down to a single field worth of overhead:
```java
@SuppressWarnings("LeakingThisInConstructor")
public ProxyLookup(Controller controller) {
data = ImmutableInternalData.EMPTY.setLookupsNoFire(
controller.setProxyLookup(this), true);
}
...
public static final class Controller {
private BiConsumer<? super Executor, ? super Lookup[]> consumer;
public Controller(Lookup... lookups) {
consumer = new InitialConsumer(lookups);
}
public Controller() {
consumer = new InitialConsumer();
}
public synchronized void setLookups(Executor exe, Lookup... lookups) {
consumer.accept(exe, lookups);
}
public void setLookups(Lookup... lookups) {
setLookups(null, lookups);
}
synchronized Lookup[] setProxyLookup(ProxyLookup lkp) {
if (!(this.consumer instanceof InitialConsumer)) {
throw new IllegalStateException("Attempting to use "
+ "ProxyLookup.Controller for more than one "
+ "ProxyLookup is illegal");
}
Lookup[] result = ((InitialConsumer) consumer).lookups;
this.consumer = lkp::setLookups;
return result;
}
private static class InitialConsumer implements BiConsumer<Executor, Lookup[]> {
Lookup[] lookups;
InitialConsumer() {
}
InitialConsumer(Lookup[] lookups) {
this.lookups = lookups;
}
@Override
public void accept(Executor ignored, Lookup[] lookups) {
this.lookups = Arrays.copyOf(lookups, lookups.length);
}
}
}
```
The locking, however, remains.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671024565
> short usage example in the docs
Consider using [codesnippet doclet](https://github.com/jtulach/codesnippet4javadoc/blob/master/README.md) which is integrated into NetBeans build system.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671232981
@JaroslavTulach fine with me, but needs squashing into a single commit on this branch for merging. You can do that, or I can do tomorrow before syncing up the release branch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r467526936
##########
File path: platform/openide.util.lookup/test/unit/src/org/openide/util/lookup/ProxyLookupFactoryMethodsTest.java
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.openide.util.lookup;
+
+import java.util.Arrays;
+import static java.util.Arrays.asList;
+import java.util.HashSet;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import org.openide.util.Lookup;
+import org.openide.util.LookupEvent;
+import org.openide.util.LookupListener;
+import org.openide.util.lookup.ProxyLookupFactoryMethodsTest.TThreadFactory.TThread;
+
+/**
+ *
+ * @author Tim Boudreau
+ */
+public class ProxyLookupFactoryMethodsTest {
+
+ private Consumer<Lookup[]> lookupConsumer;
+ private BiConsumer<Executor, Lookup[]> lookupBiConsumer;
+
+ private Lookup createWithSingleConsumer(Lookup... lookups) {
+ ProxyLookup.Controller controller = new ProxyLookup.Controller(lookups);
+ lookupConsumer = controller::setLookups;
+ return new ProxyLookup(controller);
Review comment:
Ok, see if the latest update satisfies your requirements.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r462085779
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+ /**
+ * Create a controller with an initial set of lookups that will be
+ * proxied by a {@link ProxyLookup} this controller is passed to the
+ * constructor of.
+ * @param lookups An array of lookups
+ */
+ public Controller(Lookup... lookups) {
+ consumer = new InitialConsumer(lookups);
+ }
+
+ /**
+ * Create a controller with an initially empty set of lookups to
+ * proxy to.
+ */
+ public Controller() {
+ consumer = new InitialConsumer();
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly, but the executor
+ * parameter will be ignored (nothing will be listening to it while it's
+ * in its constructor anyway).
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public synchronized void setLookups(Executor exe, Lookup... lookups) {
+ consumer.accept(exe, lookups);
Review comment:
Throw `IllegalArgumentException`, maybe. It is a bad habit to abuse `NullPointerException` for argument checking - when you see a `NullPointerException`, that should mean the JVM threw it because it was asked to do something that is impossible with a null. One case is a bad *argument* that was checked (bug in the caller), the other is a bug in the callee (at least, failure to check the input) - if you use `NullPointerException` for both cases, you lose the ability to assume `IllegalArgumentException` means *my code is probably wrong, look at that first* and `NullPointerException` means *library code may have a problem, maybe look at that first* - and that makes debugging things faster.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-654096624
> Ugh. Why, why, why do we not use Github’s issue tracking?
While I share a disdain for JIRA, being able to archive everything fully at ASF is important. eg. all the old bugzilla links are still working hosted at ASF.
> Rather than using Consumer and BiConsumer, I'd suggest to create inner class Controller in ProxyLookup and add constructor that takes the Controller.
+1 for a nicer and more easily understandable API.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r449888927
##########
File path: platform/openide.util.lookup/apichanges.xml
##########
@@ -53,7 +53,7 @@
</p>
</description>
<class name="ProxyLookup" package="org.openide.util.lookup"/>
- <issue number="pending"/>
+ <issue number="NETBEANS-2232"/>
Review comment:
There is no chicken and egg problem, because this should be a JIRA ticket. AFAIK this will currently get linked to https://issues.apache.org/jira/browse/NETBEANS-2232
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671119365
Re running `ant commit-validation`, which I am trying to do with my branch that actually uses these patches - I do not know what to make [of these failures](https://timboudreau.com/files/nb-build-failure/) (that's a link to the html report). Is the `commit-validation` suite unmaintained? Should I have built a smaller cluster config than `full`? This is hard to make heads or tails of, since none of them _look_ like related failures, but given that we're changing really low-level, pervasively called code in `Lookup.getDefault()` and `FileObject.getLookup()`, a second pair of eyes on it would be good.
The branch [in question is here](https://github.com/timboudreau/incubator-netbeans/tree/proxy-lookup-factory-methods-impl)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r467566257
##########
File path: platform/openide.util.lookup/apichanges.xml
##########
@@ -53,7 +53,7 @@
</p>
</description>
<class name="ProxyLookup" package="org.openide.util.lookup"/>
- <issue number="pending"/>
+ <issue number="NETBEANS-2232"/>
Review comment:
Fixed in 87a3727e26
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-654048910
Rather than using `Consumer` and `BiConsumer`, I'd suggest to create inner class `Controller` in `ProxyLookup` and add constructor that takes the `Controller`. The `Controller` would have public `setLookups` method. Kind of similar to `AbstractLookup` and its `Content`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671113784
The continuous build failure looks pretty completely unrelated - an [NPE deep in javac](https://timboudreau.com/files/screen/08-09-2020_19-25-05.png)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-655916926
It should work the same way [AbstractLookup.Content](https://bits.netbeans.org/12.0/javadoc/org-openide-util-lookup/org/openide/util/lookup/AbstractLookup.Content.html) does - e.g. give the creator of the `ProxyLookup` additional privileges by keeping a an internal reference to `Content` and giving out `ProxyLookup` to everyone.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-653994647
Ugh. Why, why, why do we not use Github’s issue tracking?
--
http://timboudreau.com
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-656783238
I'm not sure I'm in love with that solution for the following reason:
ProxyLookup.Controller ctrl = new ProxyLookup.Controller();
ProxyLookup lookup1 = new Proxylookup(ctrl);
ProxyLookup lookup2 = new ProxyLookup(ctrl);
and what happens?
Not to mention that ProxyLookup.Controller will need internal fields to
hold any lookups set before it has been attached to a ProxyLookup, adding
to the footprint unnecessarily.
Seems clunkier while adding little value.
-Tim
On Thu, Jul 9, 2020 at 1:53 AM Jaroslav Tulach <no...@github.com>
wrote:
> It should work the same way AbstractLookup.Content
> <https://bits.netbeans.org/12.0/javadoc/org-openide-util-lookup/org/openide/util/lookup/AbstractLookup.Content.html>
> does - e.g. give the creator of the ProxyLookup additional privileges by
> keeping a an internal reference to Content and giving out ProxyLookup to
> everyone.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/netbeans/pull/2232#issuecomment-655916926>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAOQE5ZHORJJGMRFZT5YW3LR2VLL3ANCNFSM4OQXM5SQ>
> .
>
--
http://timboudreau.com
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671159898
> Do we really need this in 12.1?
No, I just wanted to reward Tim for polishing his PR. Up to you, as release coordinator, to decide.
The integration of this PR shall be safe. Only when we try to [use of the new API](https://github.com/timboudreau/incubator-netbeans/tree/proxy-lookup-factory-methods-impl) we can get failures. That [branch](https://github.com/timboudreau/incubator-netbeans/tree/proxy-lookup-factory-methods-impl) is clearly for post 12.1.
> Is the API documentation correct?
It wasn't. I've done some necessary and some unnecessary changes in 316acb3 and 9e2f30f
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671039920
Thanks, although I didn't say we needed a JIRA ticket, just that if there's an issue link it needs to go there. Have added to title, which may or may not link things up - it's meant to sync.
Do we really need this in 12.1? I'm trying to get last PRs synced for final beta over next couple of days. We're in feature freeze, but not against this if it's problem free! It needs squashing and force pushing here before merging though (GitHub squash and merge is problematic).
Is the API documentation correct?
> if called before that ProxyLookup has been created, the lookup contents will be set during that ProxyLookup's constructor as if you had passed them to the constructor directly.
Am I missing something? Doesn't it throw an IllegalStateException in that scenario?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
jtulach commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r463406452
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+ /**
+ * Create a controller with an initial set of lookups that will be
+ * proxied by a {@link ProxyLookup} this controller is passed to the
+ * constructor of.
+ * @param lookups An array of lookups
+ */
+ public Controller(Lookup... lookups) {
+ consumer = new InitialConsumer(lookups);
+ }
+
+ /**
+ * Create a controller with an initially empty set of lookups to
+ * proxy to.
+ */
+ public Controller() {
+ consumer = new InitialConsumer();
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly, but the executor
+ * parameter will be ignored (nothing will be listening to it while it's
+ * in its constructor anyway).
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public synchronized void setLookups(Executor exe, Lookup... lookups) {
+ consumer.accept(exe, lookups);
Review comment:
Throw whatever you prefer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] asfgit merged pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
jtulach commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r463406060
##########
File path: platform/openide.util.lookup/test/unit/src/org/openide/util/lookup/ProxyLookupFactoryMethodsTest.java
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.openide.util.lookup;
+
+import java.util.Arrays;
+import static java.util.Arrays.asList;
+import java.util.HashSet;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import org.openide.util.Lookup;
+import org.openide.util.LookupEvent;
+import org.openide.util.LookupListener;
+import org.openide.util.lookup.ProxyLookupFactoryMethodsTest.TThreadFactory.TThread;
+
+/**
+ *
+ * @author Tim Boudreau
+ */
+public class ProxyLookupFactoryMethodsTest {
+
+ private Consumer<Lookup[]> lookupConsumer;
+ private BiConsumer<Executor, Lookup[]> lookupBiConsumer;
+
+ private Lookup createWithSingleConsumer(Lookup... lookups) {
+ ProxyLookup.Controller controller = new ProxyLookup.Controller(lookups);
+ lookupConsumer = controller::setLookups;
+ return new ProxyLookup(controller);
Review comment:
I believe the test should show the expected usage of the API which is now without `BiConsumer` & co. Keeping such interface in the implementation or test is a relic of an API which disappeared.
I am running `Lookup` & co. in restricted environments (see for example #2226). Increased usage of JDK8 APIs complicates that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-656788918
Not to mention that it necessarily introduces some locking into the picture, since using the controller and setting the proxy lookup is a potential race:
```java
public static final class Controller {
private Lookup[] initialLookups;
private Executor initialExecutor;
private ProxyLookup proxyLookup;
public Controller(Lookup... lookups) {
initialLookups = lookups;
}
public Controller() {
initialLookups = new Lookup[0];
}
public synchronized void setLookups(Executor exe, Lookup... lookups) {
if (proxyLookup != null) {
proxyLookup.setLookups(exe, lookups);
} else {
initialLookups = Arrays.copyOf(lookups, lookups.length);
initialExecutor = exe;
}
}
public void setLookups(Lookup... lookups) {
setLookups(null, lookups);
}
synchronized void setProxyLookup(ProxyLookup lkp) {
if (this.proxyLookup != null) {
throw new IllegalStateException("Attempting to use "
+ "ProxyLookup.Controller for more than one "
+ "ProxyLookup is illegal");
}
lkp.setLookups(initialExecutor, initialLookups);
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671025075
I do not have commit rights in the Apache repositories - someone else will need to merge.
I have another branch with patches to use this code running tests now. The full test suites for lookup, filesystems and masterfs pass; loaders has one failure, but it is a test marked @RandomlyFails. Tests in the properties module and a few other places that are affected seem to be unmaintained, and fail in ways unlikely to be connected to the patches.
Is the commit-validation suite still maintained / reliable?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671105098
> Am I missing something? Doesn't it throw an IllegalStateException in that scenario?
It does now, per Jarda's request. If I weren't at this moment attempting once again to run tests against a sub-branch that modifies a bunch of modules (including the default lookup itself) to use this patch. Feel free to fix in the meantime, or I'll get to it this evening, once my laptop isn't being brought to its knees by tests.
This ought to be perfectly safe for the current release - nothing uses the code without the patches I'm going to submit as a separate pull request once I'm sure they don't break anything. I, for one, have two large NetBeans-based codebases I would immediately begin using ProxyLookup.Controller in if it were available, with probably 10-12 ProxyLookup subclasses that could be eliminated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau edited a comment on pull request #2232: [NETBEANS-4699] Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau edited a comment on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671105098
> Am I missing something? Doesn't it throw an IllegalStateException in that scenario?
It does now, per Jarda's request. If I weren't at this moment attempting once again to run tests against a sub-branch that modifies a bunch of modules (including the default lookup itself) to use this patch I'd fix it right now. Feel free to fix in the meantime, or I'll get to it this evening, once my laptop isn't being brought to its knees by tests.
This ought to be perfectly safe for the current release - nothing uses the code without the patches I'm going to submit as a separate pull request once I'm sure they don't break anything. I, for one, have two large NetBeans-based codebases I would immediately begin using ProxyLookup.Controller in if it were available, with probably 10-12 ProxyLookup subclasses that could be eliminated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-655862811
If you actually mean *inner* class, how does that work? You construct it
before you’ve instantiated the outer instance?
I guess you mean Controller has a package or private access method called
by the proxy lookup that takes the biconsumer and delegates to it?
On Mon, Jul 6, 2020 at 2:51 AM Jaroslav Tulach <no...@github.com>
wrote:
> Rather than using Consumer and BiConsumer, I'd suggest to create inner
> class Controller in ProxyLookup and add constructor that takes the
> Controller. The Controller would have public setLookups method. Kind of
> similar to AbstractLookup and its Content.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/netbeans/pull/2232#issuecomment-654048910>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAOQE545SF5JRIFW5E7GCRDR2FYAJANCNFSM4OQXM5SQ>
> .
>
--
http://timboudreau.com
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach edited a comment on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach edited a comment on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671035792
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-671035792
I see. I created [NETBEANS-4699|https://issues.apache.org/jira/browse/NETBEANS-4699] issue to satisfy Neil's request. I believe this PR can be merged. Can I do it now? It is a safe, compatible API addition.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r453354820
##########
File path: platform/openide.util.lookup/apichanges.xml
##########
@@ -25,6 +25,33 @@
<apidef name="lookup">Lookup API</apidef>
</apidefs>
<changes>
+ <change id="ProxyLookupController">
+ <api name="lookup"/>
+ <summary>Add ProxyLookup.Controller to set lookups dynamically without
+ subclassing</summary>
+ <version major="8" minor="43"/>
+ <date year="2020" month="7" day="4"/>
+ <author login="tboudreau"/>
+ <compatibility addition="yes" source="compatible" semantic="compatible" binary="compatible"/>
+ <description>
+ <p>
+ One of the most common uses of ProxyLookup is to dynamically
+ change the set of lookups being delegated to. The objection
+ to exposing the <code>setLookups()</code> method is straightforward -
+ it would be inviting <i>clients</i> of such a Lookup to call it.
+ </p><p>
+ A cursory grep of NetBeans' own sources finds <b>53 such subclasses</b>
Review comment:
Rather than describing usage of `grep`, I suggest to make the changes in the overall NetBeans sources, once we agree on the API.
##########
File path: platform/openide.util.lookup/test/unit/src/org/openide/util/lookup/ProxyLookupFactoryMethodsTest.java
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.openide.util.lookup;
+
+import java.util.Arrays;
+import static java.util.Arrays.asList;
+import java.util.HashSet;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import org.openide.util.Lookup;
+import org.openide.util.LookupEvent;
+import org.openide.util.LookupListener;
+import org.openide.util.lookup.ProxyLookupFactoryMethodsTest.TThreadFactory.TThread;
+
+/**
+ *
+ * @author Tim Boudreau
+ */
+public class ProxyLookupFactoryMethodsTest {
+
+ private Consumer<Lookup[]> lookupConsumer;
+ private BiConsumer<Executor, Lookup[]> lookupBiConsumer;
+
+ private Lookup createWithSingleConsumer(Lookup... lookups) {
+ ProxyLookup.Controller controller = new ProxyLookup.Controller(lookups);
+ lookupConsumer = controller::setLookups;
+ return new ProxyLookup(controller);
Review comment:
This would be replaced with:
```java
ProxyLookup.Controller controller = new ProxyLookup.Controller();
ProxyLookup l = new ProxyLookup(controller);
l.setLookups(lookups);
return l;
```
I suggest to remove `BiConsumer` altogether as the test should mimic the API usage - e.g. directly use the `Controller`.
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -57,6 +58,26 @@
public ProxyLookup(Lookup... lookups) {
data = ImmutableInternalData.EMPTY.setLookupsNoFire(lookups, true);
}
+ /**
+ * Create a ProxyLookup whose contents can be set dynamically without
+ * exposing the setter to consumers of the lookup; the passed
Review comment:
The passed in `Consumer` is no longer called back. It can:
"be used later to change the set of lookups the `ProxyLookup` delegates to via `Controller.setLookups` method"
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
Review comment:
I really want this change to not rely on JDK8 APIs.
I suggest to change this field to `ProxyLookup`.
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+ /**
+ * Create a controller with an initial set of lookups that will be
+ * proxied by a {@link ProxyLookup} this controller is passed to the
+ * constructor of.
+ * @param lookups An array of lookups
+ */
+ public Controller(Lookup... lookups) {
+ consumer = new InitialConsumer(lookups);
+ }
+
+ /**
+ * Create a controller with an initially empty set of lookups to
+ * proxy to.
+ */
+ public Controller() {
+ consumer = new InitialConsumer();
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly, but the executor
+ * parameter will be ignored (nothing will be listening to it while it's
+ * in its constructor anyway).
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public synchronized void setLookups(Executor exe, Lookup... lookups) {
+ consumer.accept(exe, lookups);
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly.
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public void setLookups(Lookup... lookups) {
+ setLookups(null, lookups);
+ }
+
+ synchronized Lookup[] setProxyLookup(ProxyLookup lkp) {
+ if (!(this.consumer instanceof InitialConsumer)) {
+ throw new IllegalStateException("Attempting to use "
+ + "ProxyLookup.Controller for more than one "
+ + "ProxyLookup is illegal");
+ }
+ Lookup[] result = ((InitialConsumer) consumer).lookups();
+ this.consumer = lkp::setLookups;
Review comment:
```
if (this.consumer != null) {
throw ISE();
}
this.consumer = lkp;
```
is all you need, in my opinion.
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+ /**
+ * Create a controller with an initial set of lookups that will be
+ * proxied by a {@link ProxyLookup} this controller is passed to the
+ * constructor of.
+ * @param lookups An array of lookups
+ */
+ public Controller(Lookup... lookups) {
Review comment:
I don't see a value in this constructor. I suggest to remove it.
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+ /**
+ * Create a controller with an initial set of lookups that will be
+ * proxied by a {@link ProxyLookup} this controller is passed to the
+ * constructor of.
+ * @param lookups An array of lookups
+ */
+ public Controller(Lookup... lookups) {
+ consumer = new InitialConsumer(lookups);
+ }
+
+ /**
+ * Create a controller with an initially empty set of lookups to
+ * proxy to.
+ */
+ public Controller() {
+ consumer = new InitialConsumer();
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly, but the executor
+ * parameter will be ignored (nothing will be listening to it while it's
+ * in its constructor anyway).
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public synchronized void setLookups(Executor exe, Lookup... lookups) {
+ consumer.accept(exe, lookups);
Review comment:
Throw `NullPointerException` if the `consumer` field isn't yet set and document that as expected behavior. That's what I'd suggest.
##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
}
return map.keySet();
}
-
+
+ /**
+ * A controller which allows the set of lookups being proxied to be
+ * set dynamically.
+ *
+ * @since 8.43
+ */
+ public static final class Controller {
+
+ private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+ /**
+ * Create a controller with an initial set of lookups that will be
+ * proxied by a {@link ProxyLookup} this controller is passed to the
+ * constructor of.
+ * @param lookups An array of lookups
+ */
+ public Controller(Lookup... lookups) {
+ consumer = new InitialConsumer(lookups);
+ }
+
+ /**
+ * Create a controller with an initially empty set of lookups to
+ * proxy to.
+ */
+ public Controller() {
+ consumer = new InitialConsumer();
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly, but the executor
+ * parameter will be ignored (nothing will be listening to it while it's
+ * in its constructor anyway).
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public synchronized void setLookups(Executor exe, Lookup... lookups) {
+ consumer.accept(exe, lookups);
+ }
+
+ /**
+ * Set the lookups on the {@link ProxyLookup} this controller controls;
+ * if called before that ProxyLookup has been created, the lookup
+ * contents will be set during that ProxyLookup's constructor as if you
+ * had passed them to the constructor directly.
+ *
+ * @param exe An executor to notify in
+ * @param lookups An array of Lookups to be proxied
+ */
+ public void setLookups(Lookup... lookups) {
+ setLookups(null, lookups);
+ }
+
+ synchronized Lookup[] setProxyLookup(ProxyLookup lkp) {
+ if (!(this.consumer instanceof InitialConsumer)) {
+ throw new IllegalStateException("Attempting to use "
+ + "ProxyLookup.Controller for more than one "
+ + "ProxyLookup is illegal");
+ }
+ Lookup[] result = ((InitialConsumer) consumer).lookups();
+ this.consumer = lkp::setLookups;
+ return result;
+ }
+
+ private static class InitialConsumer implements BiConsumer<Executor, Lookup[]> {
Review comment:
Remove this class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] timboudreau commented on pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing
Posted by GitBox <gi...@apache.org>.
timboudreau commented on pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#issuecomment-656892598
Hrm, just got a note that tests failed with this patch, but it seems completely unrelated to it:
`The template is not valid. .github/workflows/main.yml (Line: 233, Col: 16): hashFiles('**/external/binaries-list') couldn't finish within 120 seconds.`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists