10:48 a.m.
As Brett finishes up the "unified schema" work I wanted to start discussion
about some simplifications that could/should come from that.
Jandex, scanning and discovery
One of the outcomes of moving to Jandex to process annotations is the
requirement that we have a good Jandex Index available to us. There are 2
parts to this:
1. We need a "baseline Index". This can be a Jandex Index passed to us
(by WildFly, e.g.). But in cases where we are not passed one, we need to
build one.
2. We then augment the Index with "virtual annotations" built from XML
mappings.
As I said, in the cases we are not handed a Jandex Index we need to build
one. Currently this is a tedious process handled by manually indexing all
known classes (known from a few different sources). But this need to know
the classes to index them is problematic.
Which got me to thinking that it sure would be nice to tie this in to the
scanning (org.hibernate.jpa.boot.scan.spi.Scanner) stuff we do already such
that as we walk the "archive" we automatically add classes we encounter to
the Index. I'd actually add things to the Index whether or not they are
supposed to be managed classes (exclude-unlisted-classes).
There are lots of benefits to this. And not just to ORM, provided we start
sharing the Jandex with Search (etc) as we have discussed. This better
ensures that all needed classes are available in the Index. This allows
better discovery of "ancillary" classes via annotation. Etc...
Technically this one is not tied to the unified schema work, we could do
this change today. The difference is that the move to the unified schema
means that everything is processed using Jandex, so having that good Index
becomes even more important.
There is one difficulty I see here. Assuming we are to only use
listed-classes, that means classes explicitly listed in the PU *and*
classes listed in XML. Checking that a class is listed in the PU is
simple, but checking whether a class comes from XML before we use it is not
as straightforward. Its not impossible, just another thing to consider.
Applying XML defaults
orm.xsd allows specifying attribute specific defaults. One such default,
e.g., is access. What happens in the current annotation "mocking"[1] is
that the default access is only applied to attributes that are referenced
in the XML. If the XML is used in an "override" way and an attribute is
defined in the entity with annotations but is not mentioned in the XML, the
default access from the XML is not picked up, as it should be (unless the
annotations define a more local explicit access).
I think the solution is to expose all the 'persistence-unit-defaults'
values on the org.hibernate.metamodel.source.spi.MappingDefaults and grab
them from there.
-- seems to me there were others I wanted to discuss, but they escape my
mind at the moment ;)
[1] I think we all dislike the term "mocking" here. I'd like to start
discussing alternatives. I mentioned "augmentating" above, I like that
one.
3:04 p.m.
On 21 Jan 2014, at 16:48, Steve Ebersole <steve(a)hibernate.org> wrote:
As Brett finishes up the "unified schema" work I wanted to
start discussion
about some simplifications that could/should come from that.
I really like where all these work is going. I hope the unification will work
As I said, in the cases we are not handed a Jandex Index we need to
build
one. Currently this is a tedious process handled by manually indexing all
known classes (known from a few different sources). But this need to know
the classes to index them is problematic.
+1 In this area we had (or maybe still have some failing tests). The old code
used to take used to take the configured classes from the config, build
the hierarchy and process all classes in the hierarchy. So it would for example
“discover” a @MappedSuperclass even so the class was not explicitly listed
in the the persistence unit / hibernate config.
When doing the initial work on Jandex I assumed that all annotated classes
are in the Jandex index. This, however, requires that all annotated classes would
need to be explicitly configured. To keep backwards compatibility we added some
code which would also do the processing of the hierarchy.
Which got me to thinking that it sure would be nice to tie this in to
the
scanning (org.hibernate.jpa.boot.scan.spi.Scanner) stuff we do already such
that as we walk the "archive" we automatically add classes we encounter to
the Index. I'd actually add things to the Index whether or not they are
supposed to be managed classes (exclude-unlisted-classes).
+1 Sounds like a good idea and also solves the problem described above in a nice
way.
There are lots of benefits to this. And not just to ORM, provided we
start
sharing the Jandex with Search (etc) as we have discussed.
+1 This would be very useful for HSEARCH-1213 where the aim is to switch to Jandex.
This better
ensures that all needed classes are available in the Index. This allows
better discovery of "ancillary" classes via annotation. Etc…
+1
I think the solution is to expose all the
'persistence-unit-defaults'
values on the org.hibernate.metamodel.source.spi.MappingDefaults and grab
them from there.
+1
—Hardy
5:25 p.m.
I have this working, but given the current scope of Scanner it is
unfortunately of limited use. That is, currently it only works when using
JPA boostrapping (technically, whenever you use the
EntityManagerFactoryBuilder).
Given how central we are making Jandex in this scheme, I think its
worthwhile to consider ways to make it work in "native Hibernate"
bootstrapping as well. Maybe this is as simple as allowing the user to
specify a base url for scanning.
I'd like to suggest one change in scanning to support this as well.
Currently when the Scanner finds a Class, or package-info, etc it
immediately checks the "options" to see if this entry can be accepted
(essentially according to the PU). I'd rather change this up so that the
collector is notified as soon as we find an entry. This would allow the
collector to index the entry in Jandex no matter what and to decide what
else to do with it (add it list of managed classes, etc).
WDYT?
On Mon, Apr 21, 2014 at 2:04 PM, Hardy Ferentschik <hardy(a)hibernate.org>wrote:
On 21 Jan 2014, at 16:48, Steve Ebersole <steve(a)hibernate.org> wrote:
> As Brett finishes up the "unified schema" work I wanted to start
discussion
> about some simplifications that could/should come from that.
I really like where all these work is going. I hope the unification will
work
> As I said, in the cases we are not handed a Jandex Index we need to build
> one. Currently this is a tedious process handled by manually indexing
all
> known classes (known from a few different sources). But this need to
know
> the classes to index them is problematic.
+1 In this area we had (or maybe still have some failing tests). The old
code
used to take used to take the configured classes from the config, build
the hierarchy and process all classes in the hierarchy. So it would for
example
“discover” a @MappedSuperclass even so the class was not explicitly listed
in the the persistence unit / hibernate config.
When doing the initial work on Jandex I assumed that all annotated classes
are in the Jandex index. This, however, requires that all annotated
classes would
need to be explicitly configured. To keep backwards compatibility we
added some
code which would also do the processing of the hierarchy.
> Which got me to thinking that it sure would be nice to tie this in to the
> scanning (org.hibernate.jpa.boot.scan.spi.Scanner) stuff we do already
such
> that as we walk the "archive" we automatically add classes we encounter
to
> the Index. I'd actually add things to the Index whether or not they are
> supposed to be managed classes (exclude-unlisted-classes).
+1 Sounds like a good idea and also solves the problem described above in
a nice
way.
> There are lots of benefits to this. And not just to ORM, provided we
start
> sharing the Jandex with Search (etc) as we have discussed.
+1 This would be very useful for HSEARCH-1213 where the aim is to switch
to Jandex.
> This better
> ensures that all needed classes are available in the Index. This allows
> better discovery of "ancillary" classes via annotation. Etc…
+1
> I think the solution is to expose all the 'persistence-unit-defaults'
> values on the org.hibernate.metamodel.source.spi.MappingDefaults and grab
> them from there.
+1
—Hardy
5:34 p.m.
On 24 Jan 2014, at 23:25, Steve Ebersole <steve(a)hibernate.org> wrote:
I'd like to suggest one change in scanning to support this as
well. Currently when the Scanner finds a Class, or package-info, etc it immediately
checks the "options" to see if this entry can be accepted (essentially according
to the PU). I'd rather change this up so that the collector is notified as soon as we
find an entry. This would allow the collector to index the entry in Jandex no matter what
and to decide what else to do with it (add it list of managed classes, etc).
+1, as you say, I think we get more benefit out of the Jandex index if we index all
entries/classes.
—Hardy
12:57 p.m.
So here is what I propose more concretely...
As a refresher, there are a few main characters:
* Scanner - coordinates the scanning effort
* PersistenceUnitDescriptor - provides access to needed PU information
* ScanOptions - a few booleans to control processing within Scanner
* ArchiveDescriptorFactory - essentially acts as a "URL interpreter" (this
is one of the main customization points in the new scanning code)
* ArchiveDescriptor / ArchiveEntry - describe the parts of the archive we
want to look at
* ArchiveEntryHandler - the main visitation contract for inspecting
ArchiveEntry
ArchiveEntryHandler is the place where the collection happens. There are
ArchiveEntryHandler specializations for each type of archive entry: class
file, package-info file, etc. The ArchiveEntryHandler for class files is
really the only place that needs a change (to add to a supplied Jandex
Indexer). However, I think some other changes are worthwhile. First,
relying on PersistenceUnitDescriptor ties this to building of an EMF. With
a simple tweak, we could make this usable in "native SF" building also.
And given the above stated importance on Jandex, I think this is
worthwhile change.
I wanted to go over the specifics because this is an SPI, and because I
know at least one customization of this SPI for integration : WildFly,
which hooks in mainly to deal with its VFS URL handling. I think WildFly
usage should be safe from these changes.
On Thu, Apr 24, 2014 at 4:34 PM, Hardy Ferentschik <hardy(a)hibernate.org>wrote:
On 24 Jan 2014, at 23:25, Steve Ebersole <steve(a)hibernate.org> wrote:
> I'd like to suggest one change in scanning to support this as well.
Currently when the Scanner finds a Class, or package-info, etc it
immediately checks the "options" to see if this entry can be accepted
(essentially according to the PU). I'd rather change this up so that the
collector is notified as soon as we find an entry. This would allow the
collector to index the entry in Jandex no matter what and to decide what
else to do with it (add it list of managed classes, etc).
+1, as you say, I think we get more benefit out of the Jandex index if we
index all entries/classes.
—Hardy
11:20 a.m.
Almost done with this. We are much more aggressive about indexing stuff
into Jandex now (which is a good thing) when we are not handed a Jandex
Index.
However, this does mean we need to be more careful in the case of JPA and
exclude-non-listed-classes. ATM we drive annotations based on Jandex (aka,
the classes known to Jandex). However, if we know index classes that
should not be used as entities, etc (because of exclude-non-listed-classes)
we are breaking the JPA spec. To this end, I suggest that scanning:
1) Index everything
2) Keep a running tab of "allowable managed classes and packages".
Later, when beginning interpretation of annotations (via Jandex) we can
cross-reference that with the list of allowable managed classes. Currently
we do:
for ( ClassInfo classInfo :
bindingContext.getJandexAccess().getIndex().getKnownClasses() ) {
// use them all...
}
What I am suggesting is:
interface ManagedClassFilter {
public boolean allowAsManagedClass(ClassInfo classInfo);
}
for ( ClassInfo classInfo :
bindingContext.getJandexAccess().getIndex().getKnownClasses() ) {
if ( !managedClassFiler.allowAsManagedClass( classInfo ) ) {
continue;
}
}
Further, rather than maintaining potentially large lists of allowable class
and package names, I'd also suggest we recognize the cases when we have
open-ended discovery in play and simply use an "all inclusive" strategy.
On Fri, Apr 25, 2014 at 11:57 AM, Steve Ebersole <steve(a)hibernate.org>wrote:
So here is what I propose more concretely...
As a refresher, there are a few main characters:
* Scanner - coordinates the scanning effort
* PersistenceUnitDescriptor - provides access to needed PU information
* ScanOptions - a few booleans to control processing within Scanner
* ArchiveDescriptorFactory - essentially acts as a "URL interpreter" (this
is one of the main customization points in the new scanning code)
* ArchiveDescriptor / ArchiveEntry - describe the parts of the archive we
want to look at
* ArchiveEntryHandler - the main visitation contract for inspecting
ArchiveEntry
ArchiveEntryHandler is the place where the collection happens. There are
ArchiveEntryHandler specializations for each type of archive entry: class
file, package-info file, etc. The ArchiveEntryHandler for class files is
really the only place that needs a change (to add to a supplied Jandex
Indexer). However, I think some other changes are worthwhile. First,
relying on PersistenceUnitDescriptor ties this to building of an EMF. With
a simple tweak, we could make this usable in "native SF" building also.
And given the above stated importance on Jandex, I think this is
worthwhile change.
I wanted to go over the specifics because this is an SPI, and because I
know at least one customization of this SPI for integration : WildFly,
which hooks in mainly to deal with its VFS URL handling. I think WildFly
usage should be safe from these changes.
On Thu, Apr 24, 2014 at 4:34 PM, Hardy Ferentschik <hardy(a)hibernate.org>wrote:
>
> On 24 Jan 2014, at 23:25, Steve Ebersole <steve(a)hibernate.org> wrote:
>
> > I'd like to suggest one change in scanning to support this as well.
> Currently when the Scanner finds a Class, or package-info, etc it
> immediately checks the "options" to see if this entry can be accepted
> (essentially according to the PU). I'd rather change this up so that the
> collector is notified as soon as we find an entry. This would allow the
> collector to index the entry in Jandex no matter what and to decide what
> else to do with it (add it list of managed classes, etc).
>
> +1, as you say, I think we get more benefit out of the Jandex index if we
> index all entries/classes.
>
> —Hardy
>
>
2:09 p.m.
On 28 Jan 2014, at 17:20, Steve Ebersole <steve(a)hibernate.org> wrote:
Almost done with this. We are much more aggressive about indexing
stuff into Jandex now (which is a good thing) when we are not handed a Jandex Index.
However, this does mean we need to be more careful in the case of JPA and
exclude-non-listed-classes. ATM we drive annotations based on Jandex (aka, the classes
known to Jandex). However, if we know index classes that should not be used as entities,
etc (because of exclude-non-listed-classes) we are breaking the JPA spec. To this end, I
suggest that scanning:
1) Index everything
2) Keep a running tab of "allowable managed classes and packages".
Later, when beginning interpretation of annotations (via Jandex) we can cross-reference
that with the list of allowable managed classes. Currently we do:
for ( ClassInfo classInfo : bindingContext.getJandexAccess().getIndex().getKnownClasses()
) {
// use them all...
}
What I am suggesting is:
interface ManagedClassFilter {
public boolean allowAsManagedClass(ClassInfo classInfo);
}
for ( ClassInfo classInfo : bindingContext.getJandexAccess().getIndex().getKnownClasses()
) {
if ( !managedClassFiler.allowAsManagedClass( classInfo ) ) {
continue;
}
}
Further, rather than maintaining potentially large lists of allowable class and package
names, I'd also suggest we recognize the cases when we have open-ended discovery in
play and simply use an "all inclusive" strategy.
sounds good.
1:30 p.m.
I am contemplating something very drastic here. The back story is that we
end up needing to index stuff from 2 places: the
EntityManagerFactoryBuilder and again from MetadataBuildingProcess (part of
Binder processing). The indexing from EntityManagerFactoryBuilder happens
just during JPA bootstrapping. The indexing from MetadataBuildingProcess
happens for both. The difficulty this presents is that we end up needing
to pass the same Indexer to both.
What I started thinking would be better is to perform all the indexing from
MetadataBuildingProcess. But given the idea to perform a full index of all
classes as we scan, that would also mean moving scanning
into MetadataBuildingProcess. Which I do not think is a bad idea (in fact
in isolation I think its a great idea). My concern is more that atm this
is exposed as an SPI from HEM under the o.h.jpa package. I could just move
the package in total to hibernate-core, I'm just ecstatic about a o.h.jpa
package being used for stuff that is (no longer) JPA specific. But maybe
that's ok i the short term? Could also do a "ghost package" concept
(deprecate old package and have it extend from new package) to help
integrators in the short term.
I have already made scanning not dependent upon the JPA spis. Used to be
that scanning operated on the PersistenceUnitDescriptor. But really we
just need a small subset of that information to actually perform scanning:
1) what is the root url?
2) what are the non-root urls?
Not really needed for scanning, but needed in collecting the scan results:
1) list of explicitly provided class names
2) list of explicitly provided mapping file names
So bottom line... none of this is JPA specific any more. Moving scanning
to MetadataBuildingProcess allows us to centralize indexing in one place.
Thoughts?
On Mon, Apr 28, 2014 at 1:09 PM, Hardy Ferentschik <hardy(a)hibernate.org>wrote:
On 28 Jan 2014, at 17:20, Steve Ebersole <steve(a)hibernate.org> wrote:
> Almost done with this. We are much more aggressive about indexing stuff
into Jandex now (which is a good thing) when we are not handed a Jandex
Index.
>
> However, this does mean we need to be more careful in the case of JPA
and exclude-non-listed-classes. ATM we drive annotations based on Jandex
(aka, the classes known to Jandex). However, if we know index classes that
should not be used as entities, etc (because of exclude-non-listed-classes)
we are breaking the JPA spec. To this end, I suggest that scanning:
> 1) Index everything
> 2) Keep a running tab of "allowable managed classes and packages".
>
> Later, when beginning interpretation of annotations (via Jandex) we can
cross-reference that with the list of allowable managed classes. Currently
we do:
>
> for ( ClassInfo classInfo :
bindingContext.getJandexAccess().getIndex().getKnownClasses() ) {
> // use them all...
> }
>
>
> What I am suggesting is:
>
> interface ManagedClassFilter {
> public boolean allowAsManagedClass(ClassInfo classInfo);
> }
>
> for ( ClassInfo classInfo :
bindingContext.getJandexAccess().getIndex().getKnownClasses() ) {
> if ( !managedClassFiler.allowAsManagedClass( classInfo ) ) {
> continue;
> }
> }
>
> Further, rather than maintaining potentially large lists of allowable
class and package names, I'd also suggest we recognize the cases when we
have open-ended discovery in play and simply use an "all inclusive"
strategy.
sounds good.
10:01 a.m.
FYI, I created a Pull Request[1] to discuss these changes if anyone was
interested. If not I'll apply this later in the week. The commits
incorporate some other concepts such as some changes to "user directed
logging" I discussed in hibernate-dev emails. I have added some line
comments at important stuff.
[1] https://github.com/hibernate/hibernate-orm/pull/737
On Wed, Apr 30, 2014 at 12:30 PM, Steve Ebersole <steve(a)hibernate.org>wrote:
I am contemplating something very drastic here. The back story is
that we
end up needing to index stuff from 2 places: the
EntityManagerFactoryBuilder and again from MetadataBuildingProcess (part of
Binder processing). The indexing from EntityManagerFactoryBuilder happens
just during JPA bootstrapping. The indexing from MetadataBuildingProcess
happens for both. The difficulty this presents is that we end up needing
to pass the same Indexer to both.
What I started thinking would be better is to perform all the indexing
from MetadataBuildingProcess. But given the idea to perform a full index
of all classes as we scan, that would also mean moving scanning
into MetadataBuildingProcess. Which I do not think is a bad idea (in fact
in isolation I think its a great idea). My concern is more that atm this
is exposed as an SPI from HEM under the o.h.jpa package. I could just move
the package in total to hibernate-core, I'm just ecstatic about a o.h.jpa
package being used for stuff that is (no longer) JPA specific. But maybe
that's ok i the short term? Could also do a "ghost package" concept
(deprecate old package and have it extend from new package) to help
integrators in the short term.
I have already made scanning not dependent upon the JPA spis. Used to be
that scanning operated on the PersistenceUnitDescriptor. But really we
just need a small subset of that information to actually perform scanning:
1) what is the root url?
2) what are the non-root urls?
Not really needed for scanning, but needed in collecting the scan results:
1) list of explicitly provided class names
2) list of explicitly provided mapping file names
So bottom line... none of this is JPA specific any more. Moving scanning
to MetadataBuildingProcess allows us to centralize indexing in one place.
Thoughts?
On Mon, Apr 28, 2014 at 1:09 PM, Hardy Ferentschik <hardy(a)hibernate.org>wrote:
>
> On 28 Jan 2014, at 17:20, Steve Ebersole <steve(a)hibernate.org> wrote:
>
> > Almost done with this. We are much more aggressive about indexing
> stuff into Jandex now (which is a good thing) when we are not handed a
> Jandex Index.
> >
> > However, this does mean we need to be more careful in the case of JPA
> and exclude-non-listed-classes. ATM we drive annotations based on Jandex
> (aka, the classes known to Jandex). However, if we know index classes that
> should not be used as entities, etc (because of exclude-non-listed-classes)
> we are breaking the JPA spec. To this end, I suggest that scanning:
> > 1) Index everything
> > 2) Keep a running tab of "allowable managed classes and packages".
> >
> > Later, when beginning interpretation of annotations (via Jandex) we can
> cross-reference that with the list of allowable managed classes. Currently
> we do:
> >
> > for ( ClassInfo classInfo :
> bindingContext.getJandexAccess().getIndex().getKnownClasses() ) {
> > // use them all...
> > }
> >
> >
> > What I am suggesting is:
> >
> > interface ManagedClassFilter {
> > public boolean allowAsManagedClass(ClassInfo classInfo);
> > }
> >
> > for ( ClassInfo classInfo :
> bindingContext.getJandexAccess().getIndex().getKnownClasses() ) {
> > if ( !managedClassFiler.allowAsManagedClass( classInfo ) ) {
> > continue;
> > }
> > }
> >
> > Further, rather than maintaining potentially large lists of allowable
> class and package names, I'd also suggest we recognize the cases when we
> have open-ended discovery in play and simply use an "all inclusive"
> strategy.
>
> sounds good.
3640
days inactive
3655
days old
8 comments
2 participants
participants (2)
-
Hardy Ferentschik -
Steve Ebersole