On Mon 2013-03-18 8:15, Steve Ebersole wrote:
On Mon 18 Mar 2013 05:14:01 AM CDT, Emmanuel Bernard wrote:
>
>JBoss AS does use this contract so if you break it, we will need some
>kind of compatibility matrix between Hibernate and JBoss AS and EAP.
>Not unsurmountable but always a small annoyance.
>Maybe other environments also make use of this interface but I am not
>aware of them.
As far as JBoss AS, Scott has been involved in this design from the
beginning.
>I'm surprised getUnqualifiedJarName is no longer needed. I thought we
>used it as the default PU name but the current code does not use
>getUnqualifiedJarName
>anymore.
I have never seen that #getUnqualifiedJarName used aside from tests.
>We initially designed the Scanner interface to minimize the work the
>Scanner implementor has to do and keep as much of the JPA knowledge to
>HEM's code. Your design seems to require the Scanner to understand more of
>JPA including the notion of root jar and additional jar files.
There is actually very very very little "JPA knowledge" being asked
of the Scanner in my proposal. Keep in mind that in both the cases
that have surfaced so far where we actually need "custom Scanner"
both are cases where the Scanner provider is also the thing that is
handing us the root/additional jars. For EE JPA thats actually part
of the PersistenceUnitInfo contract; no magic there. So for JBoss
AS (or another AS) to hand us both the PersistenceUnitInfo (with jar
urls) and the Scanner (knowning how to scan said url protocols) is
not unreasonable. In the case of Enterprise OSGi (at least based on
our initial target environment), we have a PersistenceUnitInfo that
only points us to the root url (#getJarFileUrls returns nothing),
but this is the kind of "environment specifics" the current
implementation forces Hibernate to understand. And then, in both
cases it forces Hibernate to import and use non-standard APIs just
to do the scanning (JBoss's VirtualFile contract and quite a few
OSGi contracts). The important point I think you are missing is
that it is far more difficult asking Hibernate to understand all the
url protocol schemes in play then for the environments using those
protocols to tell use how to scan them.
That was the point of the Scanning API in the first place: not to depend
on specific APIs like the VirtualFile. That and the fact that JBoss AS
had Jandex and we did not were the reason for Ales and me to have this
interface.
>Things around:
>
>- getMappingFileNames to return the stream for these files,
Not at all following here. Do you mean getMappingFileNames on the
PersistenceUnitInfo? Well that does *not* return streams, it
returns Strings. And the spec specifically says that the Strings
are supposed to be the resource names of the mapping files (aka,
they should be loadable by that name through ClassLoader). So what
exactly is the point here?
The previous implementation of Scanner was returning the resource
streams so you had to take PUI.getMappingFileNames() into account. I got
confused.
>- isExcludeUnlistedClasses to not scan classes in the root JAR,
Exactly. This "option" only has bearing on the root jar. For all
other jars Hibernate tries to be friendly and load everything. But,
that is hardly "deep JPA knowledge". The option in terms of the
root jars maps directly to an explicit JPA discussion. Nothing deep
about the knowledge there. And for the non-root jars, there is
nothing JPA specific in this option; its purely a Hibernate
*choice*.
No that's what the JPA spec says (or intended), additional jars are to be scanned to
find the entity classes. Otherwise the additional jar feature becomes
very limited or even useless. You would only need it otherwise to
retrieve the XML DD which was considered a second class citizen on JPA 1.0.
>- getJarFileUrls
Again, I think you are missing the point that generally speaking the
PersistenceUnitInfo provider and the Scanner provider are
one-in-the-same.
>- look for META-INF/orm.xml in the root JAr (only) and exclude it if it
>is already listed explicitly in the getMappingFileNames to not process
>it twice.
Not sure how this is classified as "deep JPA knowledge".
Understanding and implementing all these rules took me a while :)
>- getManagedClassNames depending on how much you delegate to the scanner
Again, not sure how this is classified as "deep JPA knowledge". I
assume you mean because of PUI#excludeUnlistedClasses, but see that
discussion above.
>That makes me concerned about code duplication and bugs unless someone
>deep in JPA immplement all of these Scanner implementations.
So, I am really not seeing this "need for deep knowledge of JPA" on
the Scanner implementor in what I propose.
I gave you my opinion, if you think that implementing Scanner will
remain simple enough then most of my remarks are moot.
Emmanuel