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
>>>
>>>