Review request for JDK-8144919: Implement missing member handler for BeansLinker

[Attila Szegedi]

> On Jan 11, 2016, at 11:49 AM, Michael Haupt <michael.haupt at oracle.com> wrote:
> 
> Hi Attila,
> 
> lower-case thumbs up, with one remark in addition to Sundar's. In BeansLinkerTest, related to the comment "No assertion for the setter; we just expect it to silently succeed" - if it's expected to succeed, shouldn't success be tested by verifying the set value?

The point is that we’re attempting to set a property (or element) that doesn’t exist on the object, so the operation is a no-op. For the list and array arguments, we’re really just testing that no exception is thrown in either the linking or invocation mechanic. Most we could test would be that after the setter invocation, the getter still returns null. For the map argument, the outcome would be different depending on what operation permutation we’re trying; if it includes SET_ELEMENT, map would gain an element named “foo”; if it’s solely SET_PROPERTY, it would again be a no-op. I’d rather not complicate the test with this for now, but thanks for the observation.

Attila.

> 
> Best,
> 
> Michael
> 
>> Am 11.01.2016 um 05:03 schrieb Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com>:
>> 
>> * Spec. changes look good.
>> 
>> * Review comments on code changes:
>> 
>> Minor: BeansLinker.java - whitespace missing after "if"if(collectionType != CollectionType.MAP && isFixedKey)
>> and few other "if" statements too.
>> 
>> +1
>> 
>> PS. Not sure if the samples under $nashorn/samples have any dependency on (older) BeansLinker behaviour.
>> 
>> -Sundar
>> 
>> On 1/6/2016 5:08 PM, Attila Szegedi wrote:
>>> Excellent, thanks for preparing it. It looks good to me.
>>> 
>>> Attila.
>>> 
>>>> On Jan 6, 2016, at 12:28 PM, Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> specdiff for this API change is here -> http://cr.openjdk.java.net/~sundar/8144919/dynalink_specdiff/overview-summary.html
>>>> 
>>>> Thanks,
>>>> -Sundar
>>>> 
>>>> On 12/24/2015 2:01 AM, Attila Szegedi wrote:
>>>>> Please review JDK-8144919 "Implement missing member handler for BeansLinker" at <http://cr.openjdk.java.net/~attila/8144919/webrev.jdk9> for <https://bugs.openjdk.java.net/browse/JDK-8144919>
>>>>> 
>>>>> This change dependes on JDK-8144917, a review request for which immediately preceded this one. I only plan to commit the two together after they have been both reviewed.
>>>>> 
>>>>> Note that I need a CCC review for this, as it touches the Dynalink public API (details in the JIRA issue). As such, even if I get 2 reviews on this, I will not be committing either this or 8144917 until CCC approval is obtained.
>>>>> 
>>>>> Thanks,
>>>>>  Attila.
>> 
> 
> -- 
> 
> <http://www.oracle.com/>
> Dr. Michael Haupt | Principal Member of Technical Staff
> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
> Oracle Java Platform Group | LangTools Team | Nashorn
> Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany
> <http://www.oracle.com/commitment>	Oracle is committed to developing practices and products that help protect the environment
>