[hibernate-dev] Scanner contract

[Steve Ebersole]

Also, I think another reason that the current scanning code is so 
complicated in the mapping file case is because we are not "just 
scanning".  If I understand correctly, we also use the 
Scanner#getFilesInJar call as an opportunity to resolve named mappings 
files.

Not saying its a bad thing to delegate this resolution to the 
environment.  In fact delegating the resolution to the environment is 
probably the best way to avoid the duplicate results.  The reason it is 
complicated as is, is that the resolution of named mappings is done 
implicitly, rather than explicitly.  So we call off to scan for mapping 
files,

That complication, to me, is based on the understanding that "scanning" 
is all about finding additional stuff.  But what we have is that 
scanning for classes and packages is all about finding additional, 
non-named things while it is the exact opposite in terms of scanning for 
mapping files where we do both: we find (and resolve) additional, 
non-named things *and* we resolve named things.

At least that is how I read the code.


On 03/18/2013 10:40 AM, Steve Ebersole wrote:
> On Mon 18 Mar 2013 09:10:40 AM CDT, Emmanuel Bernard wrote:
>>
>> 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.
> Ok, but the decision to ask the Scanner these questions for each jar 
> is inadvertently requiring Hibernate to be sensitive to these 
> protocols.  In fact the current standard Scanner impl has explicit 
> handling for VFS protocol schemes.  And for OSGi, it would require 
> that the single root URL (because again thats all we get) actually 
> encompasses the root jar *and* any other specified jars.  Aka, `if ( 
> "bundle".equals( urlProtocol ) ) { doSomethingDrasticallyDifferent(); 
> }...`.
>
> So take the EE case, which I know is true of JBoss.  JBoss is the one 
> handing us the PersistenceUnitInfo (which is the thing that backs the  
> PersistenceUnitDescriptor we pass back to it in this Scanner 
> proposal).  I don't think it is unreasonable that JBoss understands 
> the difference between PersistenceUnitInfo#getPersistenceUnitRootUrl 
> and PersistenceUnitInfo#getJarFileUrls intrinsically (after all, it is 
> the one that gave us the PUI).  By intrinsically I mean that it 
> already understands that there is a difference/distinction between the 
> root and any non-roots.  So from there the question really is whether 
> expecting the Scanner to apply different scanning rules to root versus 
> not-root URLs constitutes "deep JPA knowledge".  And maybe that is 
> different for each type of thing for which we scan.
>
> For classes e.g. nothing stops us from saying the the Scanner just 
> returns us everything it finds and that we then filter or weed through 
> it.  For example, if it returns us descriptors for all the classes it 
> finds, we can apply the "only use that class if it (a) came from a 
> specified jar or (b) came from the root and #excludeUnlistedClasses 
> allows us to use them" logic ourselves. Thats a trivial matter of us 
> defining the ClassDescriptor contract to tell us whether the class was 
> found in the root or not:
>
> interface ClassDescriptor {
>     public String getClassName();
>     public NamedInputStream getNamedInputStream();
>     public boolean foundInRoot();
> }
>
> But again, I dont think that the Scanner applying that simple logic is 
> really "deep JPA knowledge".  But if everyone else agrees it is, we 
> can easily alleviate that via the above pattern.
>
> As for resources (orm.xml and hbm.xml files), I'd have to look a 
> little closer maybe.  To be honest that is some of the code that uses 
> the "Map as an API" approach that makes me want to rip out my hair, 
> gouge out my eyes and leap off the nearest bridge :)  It is code like 
> this code that has made me want to re-write what was EJB3Configuration 
> from scratch ever since I first started looking at it.  That and the 
> fact that any time I ask anyone that had a hand in writing that code 
> anything about that code they simply throw up their hands and say they 
> don't know ;)
>
> Maybe it is more complicated than I am making it out to be.  But I 
> don't think the way this is then presented to the rest of the code 
> needs to be complex.  At the end of the day, there should be a scan 
> and the results of that scan should be a nice easy-to-understand 
> contract.
>
>> The previous implementation of Scanner was returning the resource
>> streams so you had to take PUI.getMappingFileNames() into account. I got
>> confused.
>
> Still not following.  The proposed Scanner contract is returning 
> NamedInputStreams, which is exactly what the current Scanner returns 
> for these.
>
>> 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.
>
> Ok, but is that "deep JPA knowledge" beyond what a typical JPA 
> container developer would generally know?
>
>
>>>> - 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 :)
> Ok, don't include duplicates.. check.  ;)  Not saying the rules are 
> simplistic.  Just questioning whether they are only known to those 
> with "deep JPA knowledge".  Not so sure that is true.
>
>> I gave you my opinion, if you think that implementing Scanner will
>> remain simple enough then most of my remarks are moot.
>
> Considering I will implement the 3 main Scanner impls... ;)  But 
> seriously, I don't think delegating to Scanner just once per 
> persistence unit puts undue difficulty on the Scanner impl.
>
>
>
>