I started work on the following plan: - deprecate current "ALL_KEYS" and List passing behavior in registerInterest () - add registerInterestForAllKeys(); - add registerInterest(T... keys) - add registerInterest(Iterable<T>keys)
I might be missing something here but: With the addition of registerInterest(Iterable<T> keys), I think we would not be able to register interest a List as the key itself. A list would be iterated over due to the addition of registerInterest(Iterable<T> keys). A list in a list would be passed into registerInterest and again be iterated over. I could change the newly created registerInterest call and explicitly name it something else or are we ok with Iterables not being able to be registered as individual keys. On Mon, Nov 20, 2017 at 9:05 AM Kirk Lund <kl...@apache.org> wrote: > John's approach looks best for when you need to specify keys. > > For ALL_KEYS, what about an API that doesn't require a token or all keys: > > public void registerInterestForAllKeys(); > > On Fri, Nov 17, 2017 at 1:24 PM, Jason Huynh <jhu...@pivotal.io> wrote: > > > Thanks John for the clarification! > > > > On Fri, Nov 17, 2017 at 1:12 PM John Blum <jb...@pivotal.io> wrote: > > > > > This... > > > > > > > The Iterable version would handle any collection type by having the > > user > > > pass > > > in the iterator for the collection. > > > > > > Is not correct. > > > > > > The Collection<E> interface itself "extends" the java.lang.Iterable<E> > > > interface (see here... > > > https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html > > under > > > "*All > > > Superinterfaces*"). > > > > > > Therefore a user can simply to this... > > > > > > *List*<KeyType> keys = ... > > > > > > region.registerInterest(keys); *// calls the > > > Region.registerInterest(:Iterable<T>) method.* > > > > > > Alternatively, this would also be allowed... > > > > > > *Set*<KeyType> keys = ... > > > > > > region.registerInterest(keys); > > > > > > > > > On Fri, Nov 17, 2017 at 11:44 AM, Jason Huynh <jhu...@pivotal.io> > wrote: > > > > > > > Current idea is to: > > > > - deprecate current "ALL_KEYS" and List passing behavior in > > > > registerInterest() > > > > - add registerInterestAllKeys(); > > > > - add registerInterest(T... keys) and registerInterest(Iterable<T> > > keys) > > > > and > > > > not have one specifically for List or specific collections. > > > > > > > > The Iterable version would handle any collection type by having the > > user > > > > pass in the iterator for the collection. > > > > > > > > On Fri, Nov 17, 2017 at 11:32 AM Jacob Barrett <jbarr...@pivotal.io> > > > > wrote: > > > > > > > > > I am failing to see where registerInterest(List<T> keys) is an > issue > > > for > > > > > the key type in the region. If our region is Region<String> then I > > > would > > > > > expect registerInterest(List<String>). If the keys are unknown or a > > mix > > > > > then you should have Region<Object> and thus > > > > registerInterest(List<Object). > > > > > > > > > > I echo John's statements on VarArgs and type erasure as well as his > > > > > argument for Iterable<T>. > > > > > > > > > > Also, List<T> does not restrict you from List indexes. The region > > would > > > > be > > > > > Region<List<String>> with registerInterest<List<List<String>>(). > > > > > > > > > > -Jake > > > > > > > > > > > > > > > On Fri, Nov 17, 2017 at 10:04 AM John Blum <jb...@pivotal.io> > wrote: > > > > > > > > > > > Personally, I prefer the var args method (registerInterest(T... > > > keys)) > > > > > > myself. It is way more convenient if I only have a few keys when > > > > calling > > > > > > this method then to have to add the keys to a List, especially > for > > > > > testing > > > > > > purposes. > > > > > > > > > > > > But, I typically like to pair that with a > > > registerInterest(Iterable<T> > > > > > > keys) method > > > > > > as well. By having a overloaded Iterable variant, then I can > pass > > in > > > > any > > > > > > Collection type I want (which shouldn't be restricted to just > > List). > > > > It > > > > > > also is a simple matter to convert any *Collection* (i.e. *List*, > > > > *Set*, > > > > > > etc) to an array, which can be passed to the var args method. By > > > using > > > > > > List, > > > > > > you are implying that "order matters" since a List is a order > > > > collection > > > > > of > > > > > > elements. > > > > > > > > > > > > This ("*It might even cause problems of pushing in **multiple > > > different > > > > > > types.*"), regarding var args, does not even make sense. > > Technically, > > > > > > List<T> is no different. Java's type erasure essentially equates > > var > > > > > args > > > > > > too "Object..." (or Object[]) and the List<T> to List (or a List > of > > > > > > Objects, > > > > > > essentially like if you just did this... List<Object>) So, while > > the > > > > > > compiler ensures compile-time type-safety of generics, there is > no > > > > > generics > > > > > > type-safety guarantees at runtime. > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Nov 17, 2017 at 9:22 AM, Jason Huynh <jhu...@pivotal.io> > > > > wrote: > > > > > > > > > > > > > Hi Mike, > > > > > > > > > > > > > > The current support for List leads to compilation issues if the > > > > region > > > > > is > > > > > > > type constrained. However I think you are suggesting instead > of > > a > > > > var > > > > > > args > > > > > > > method, instead provide a registerInterest(List keys) method? > > > > > > > > > > > > > > So far what I am hearing requested is: > > > > > > > deprecate current "ALL_KEYS" and List passing behavior > > > > > > > registerInterestAllKeys(); > > > > > > > registerInterest(List<T> keys) instead of a > registerInterest(T... > > > > keys) > > > > > > > > > > > > > > Will anyone ever actually have a List as the key itself? The > > > current > > > > > and > > > > > > > suggested changes would not allow it registering for a specific > > > List > > > > > > > object. > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Nov 16, 2017 at 6:50 PM Jacob Barrett < > > jbarr...@pivotal.io > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Geode Native C++ and .NET have: > > > > > > > > > > > > > > > > virtual void registerKeys(const > > > > > > > > std::vector<std::shared_ptr<CacheableKey>> & keys, > > > > > > > > bool isDurable = false, > > > > > > > > bool getInitialValues = false, > > > > > > > > bool receiveValues = true) = 0; > > > > > > > > > > > > > > > > virtual void unregisterKeys(const > > > > > > > > std::vector<std::shared_ptr<CacheableKey>> & keys) = 0; > > > > > > > > > > > > > > > > virtual void *registerAllKeys*(bool isDurable = false, > > > > > > > > bool getInitialValues = false, > > > > > > > > bool receiveValues = true) = > 0; > > > > > > > > > > > > > > > > virtual void unregisterAllKeys() = 0; > > > > > > > > > > > > > > > > virtual void registerRegex(const std::string& regex, > > > > > > > > bool isDurable = false, > > > > > > > > bool getInitialValues = false, > > > > > > > > bool receiveValues = true) = 0; > > > > > > > > > > > > > > > > virtual void unregisterRegex(const char* regex) = 0; > > > > > > > > > > > > > > > > I dislike special values like this so yes please make it go > > away! > > > > > > > > > > > > > > > > -Jake > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Nov 16, 2017 at 5:20 PM Dan Smith <dsm...@pivotal.io > > > > > > wrote: > > > > > > > > > > > > > > > > > I don't really like the regex option - it implies that your > > > keys > > > > > are > > > > > > > all > > > > > > > > > strings. Will any other regular expressions work on non > > string > > > > > > objects? > > > > > > > > > registerInterestAllKeys() seems like a better option. > > > > > > > > > > > > > > > > > > -Dan > > > > > > > > > > > > > > > > > > On Thu, Nov 16, 2017 at 4:34 PM, Michael Stolz < > > > > mst...@pivotal.io> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > I don't like the vararg option. > > > > > > > > > > If i'm maintaining a list of keys i'm interested in, I > want > > > to > > > > be > > > > > > > able > > > > > > > > to > > > > > > > > > > pass that List in. > > > > > > > > > > Varargs is a poor substitute. It might even cause > problems > > of > > > > > > pushing > > > > > > > > in > > > > > > > > > > multiple different types. Keys must all be of one type > for > > a > > > > > given > > > > > > > > > Region. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm very much in favor of deprecating the ALL_KEYS string > > in > > > > > favor > > > > > > of > > > > > > > > > > something that is typed specially if you refer to > ALL_KEYS. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > If that works, then we don't necessarily need the > > additional > > > > API > > > > > > > > > > registerInterestAllKeys(). But if ALL_KEYS can't be a > > special > > > > > type > > > > > > to > > > > > > > > get > > > > > > > > > > over the compilation issues then we should go with the > new > > > API. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > Mike Stolz > > > > > > > > > > Principal Engineer, GemFire Product Lead > > > > > > > > > > Mobile: +1-631-835-4771 <(631)%20835-4771> > <(631)%20835-4771> > > > <(631)%20835-4771> <(631)%20835-4771> > > > > > <(631)%20835-4771> > > > > > > <(631)%20835-4771> > > > > > > > > > > > > > > > > > > > > On Thu, Nov 16, 2017 at 7:02 PM, Anilkumar Gingade < > > > > > > > > aging...@pivotal.io> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > +1 Deprecating ALL_KEYS option; I believe this is added > > > > before > > > > > we > > > > > > > > > > supported > > > > > > > > > > > regex support. > > > > > > > > > > > > > > > > > > > > > > Doesn't seems like a new API is needed. The regex java > > doc > > > > > > clearly > > > > > > > > > > > specifies the effect of ".*". > > > > > > > > > > > > > > > > > > > > > > +1 for deprecating list argument; and replacing with > new > > > API. > > > > > > > > > > > > > > > > > > > > > > -Anil. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Nov 16, 2017 at 3:36 PM, Jason Huynh < > > > > > jhu...@pivotal.io> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > For GEODE-3813 < > > > > > > https://issues.apache.org/jira/browse/GEODE-3813 > > > > > > > >: > > > > > > > > > > > Region > > > > > > > > > > > > registerInterest API usage of type parameters is > broken > > > > > > > > > > > > <https://issues.apache.org/jira/browse/GEODE-3813> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The current API to registerInterest allows a special > > > string > > > > > > token > > > > > > > > > > > > “ALL_KEYS” to be passed in as the parameter to > > > > > > registerInterest(T > > > > > > > > > key). > > > > > > > > > > > > This special token causes the registerInterest to > > behave > > > > > > similar > > > > > > > to > > > > > > > > > > > > registerInterestRegex(“.*”). As the ticket states, > if > > > the > > > > > > region > > > > > > > > has > > > > > > > > > > > been > > > > > > > > > > > > typed to anything other than Object or String, the > > usage > > > of > > > > > > > > > “ALL_KEYS” > > > > > > > > > > > as a > > > > > > > > > > > > parameter results in a compilation error. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Proposals: > > > > > > > > > > > > > > > > > > > > > > > > I would like to deprecate the special string > “ALL_KEYS” > > > and > > > > > > > > document > > > > > > > > > a > > > > > > > > > > > > workaround of using registerInterestRegex(“.*”) or we > > can > > > > > add a > > > > > > > new > > > > > > > > > API > > > > > > > > > > > > called registerInterestAllKeys() > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think we should also deprecate passing a List > Object > > of > > > > > keys > > > > > > > into > > > > > > > > > > > > registerInterest. It has the same compilation > > > restrictions > > > > > as > > > > > > > > > > “ALL_KEYS” > > > > > > > > > > > > when the region is key constrained/typed. The reason > > why > > > > > List > > > > > > > > would > > > > > > > > > be > > > > > > > > > > > > used is to allow registering multiple keys at once. > > > > Instead, > > > > > > we > > > > > > > > can > > > > > > > > > > add > > > > > > > > > > > a > > > > > > > > > > > > new var arg API like registerInterest(T… keys). This > > > > problem > > > > > > and > > > > > > > > > > > solution > > > > > > > > > > > > was also documented in the ticket by the ticket > creator > > > > (Kirk > > > > > > > Lund) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > > > -Jason > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > -John > > > > > > john.blum10101 (skype) > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > -John > > > john.blum10101 (skype) > > > > > >
