I think Dan’s suggestion clarifies the intent. Anthony
> On Nov 30, 2017, at 3:54 PM, Dan Smith <dsm...@pivotal.io> wrote: > > I think it should be registerInterestAll(Iterable<T>keys) to mirror > Collection.addAll. > > I could easily see a user creating their own Tuple class or using one that > happens to also implement Iterable as their key. > > We're not doing a very good job of discouraging people to use iterable > objects as their key if they work everywhere else and then break with this > one API. > > -Dan > > On Thu, Nov 30, 2017 at 2:45 PM, John Blum <jb...@pivotal.io> wrote: > >> No, no... good question. >> >> I just think it would be wiser if users created a single, CompositeKey >> class type, with properly implements equals and hashCode methods, as I >> pointed out. >> >> I don't see any advantage in using a java.util.Collection as a key over >> implementing a CompositeKey type. >> >> As such, anything we can do to discourage users from using Collection types >> as a key, I think is a good thing. >> >> >> On Thu, Nov 30, 2017 at 2:35 PM, Jason Huynh <jhu...@pivotal.io> wrote: >> >>> Yeah I am not sure if anyone does it,and I don't think it would be a good >>> idea to use a collection as a key but just thought I'd ask the >> question... >>> >>> On Thu, Nov 30, 2017 at 2:33 PM John Blum <jb...@pivotal.io> wrote: >>> >>>> For instance, this came up recently... >>>> >>>> >>>> https://stackoverflow.com/questions/46551278/gemfire- >>> composite-key-pojo-as-gemfire-key >>>> >>>> I have seen other similar posts too! >>>> >>>> >>>> >>>> On Thu, Nov 30, 2017 at 2:30 PM, John Blum <jb...@pivotal.io> wrote: >>>> >>>>> Does anyone actually do this in practice? If so, yikes! >>>>> >>>>> Even if the List is immutable, the elements may not be, so using a >> List >>>> as >>>>> a key starts to open 1 up to a lot of problems. >>>>> >>>>> As others have pointed out in SO and other channels, information >> should >>>>> not be kept in the key. >>>>> >>>>> It is perfect fine to have a "Composite" Key, but then define a >>>>> CompositeKey class type with properly implemented equals(:Object) and >>>>> hashCode():int methods. >>>>> >>>>> For the most part, Keys should really only ever be simple Scalar >> values >>>>> (e.g. Long, String, etc). >>>>> >>>>> -j >>>>> >>>>> >>>>> >>>>> >>>>> On Thu, Nov 30, 2017 at 2:25 PM, Jason Huynh <jhu...@pivotal.io> >>> wrote: >>>>> >>>>>> 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<Str >>>>>> ing>>(). >>>>>>>>>>> >>>>>>>>>>> -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> >>>>>>>>>>>> <(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) >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> -John >>>>> john.blum10101 (skype) >>>>> >>>> >>>> >>>> >>>> -- >>>> -John >>>> john.blum10101 (skype) >>>> >>> >> >> >> >> -- >> -John >> john.blum10101 (skype) >>
