persistence@glassfish.java.net

Re: code review for issue #1258

From: Sanjeeb Kumar Sahoo <Sanjeeb.Sahoo_at_Sun.COM>
Date: Fri, 06 Oct 2006 23:23:11 +0530

Marina,

Good point.

Tom,

I take back my proposal to fix this issue. I shall update the issue.

Thanks,
Sahoo
Marina Vatkina wrote:
> Tom, Sahoo,
>
> I'm also concerned. Here is how I see it: it's OK to auto-detect anything
> as long as there will be no surprises for a portable application that
> lists all classes explicitly. So if to stop auto-detection, the user
> needs
> to add an unexpected attribute, it's not right. On the other hand, if no
> classes are listed, or exclude-unlisted-classes is set to false, we
> should
> be able to try auto-detection as this is already a non-portable
> application.
>
> Now about auto-detecting MappedSuperclass or Embeddable class names -
> nothing wrong will happen if the user lists them explicitly, right? We
> just help them
> if they forgot (i.e. we don't break the rules).
>
> thanks,
> -marina
>
> Sanjeeb Kumar Sahoo wrote:
>> Hi Tom,
>>
>> While we are waiting for others' comments, I just want to say a
>> couple more reasons in favor of the change:
>>
>> 1) We already support features that make persistence.xml
>> non-portable. e.g. we don't require MappedSuperclass or Embeddable
>> class names to be listed in persistence.xml. We support <jar-file> in
>> Java SE environment which is not required to be supported.
>>
>> 2) Our competitors are taking this feature for granted as the
>> following comment shows...
>> http://www.theserverside.com/news/thread.tss?thread_id=38082#193940.
>>
>> Thanks again for getting back so quickly.
>> Sahoo
>>
>> Tom Ware wrote:
>>
>>> Hi Sahoo,
>>>
>>> I am somewhat undecided about whether this change is a good idea or
>>> not.
>>>
>>> You have done a good job at pointing out the benefits of this
>>> change in the bug.
>>> (https://glassfish.dev.java.net/issues/show_bug.cgi?id=1258)
>>>
>>> The drawback of the fix is in the issue of portability. By the spec:
>>>
>>> "To insure the portability of a Java SE application, it is necessary
>>> to explicitly list
>>> the managed persistence classes that are included in the persistence
>>> unit."
>>>
>>> As a result, with this change, by default, an application that
>>> makes use of this default is not portable. Before this fix, the user
>>> had to explicitly specify something that would make their
>>> application non-portable.
>>>
>>> I am not totally opposed to this change. I just think it merits
>>> some discussion. Does anyone have any comments?
>>>
>>> Assuming we come to a consensus about the desirability of the
>>> change, the code change is fine.
>>>
>>> -Tom
>>>
>>> Sanjeeb Kumar Sahoo wrote:
>>>
>>>> Hi Tom,
>>>>
>>>> Please review the fix for
>>>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1258
>>>>
>>>> In order to test this functionality, I have also changed one of the
>>>> persistence-units in entity-persistence-tests to not specify
>>>> exclude-unlisted-classes.
>>>>
>>>> Thanks,
>>>> Sahoo
>>>>
>>>>