Re: [hibernate-dev] Hibernate Search compatibility with ORM 4.3

2013-09-05 Thread Gunnar Morling
I'm not yet understanding #1. When compiling against 4.2, wouldn't the new
methods be missing or, if you added them, refer to types not being present
with 4.2? How would this option exactly look?

For the adapters to work in this particular case, I think you'd have to
move FTSI and FTEMI to the adapter modules which does not seem so nice,
given that they are such central classes.

So I think I'd go for the proxy approach. As you say we could get rid of
the proxies once we depend on an ORM version which has this
delegating/forwarding base class. And in the long-long term we might even
benefit from Java 8 default methods for this kind of issue :)

--Gunnar



2013/9/4 Sanne Grinovero 

> That's one possibility, but to recap all options discussed so far:
>
> One alternative which I consider quite tempting is that we don't
> implement the new methods defined in Session and EntityManager (JPA
> 2.1), but keeping it fully compatible in all other ways.
>
> We have this option because I resolved all remaining issues and could
> run the testsuite succesfully on both ORM 4.2 and 4.3, with these
> exceptions:
>  - needs to be compiled with ORM 4.3 to have access to the new types
> used in the new method signatures
>  - when an application is deployed on JBoss EAP 6.1 or 6.0 it fails
> hard as it eagerly realizes that some types aren't availabled
>
> In other words that means that Hibernate Search 4.4 users would
> benefit from improvements in latest ORM but not from the new JPA
> features.
>
> Our options:
>
> 1# Support ORM 4.2
> If we compile against ORM 4.2 with these patches, it will work on ORM
> 4.3 as well, as long as the new methods aren't used.
>
> 2# Gunnar's adapters
> have two separate Maven modules containing the two different adaptors
> needed, and bundle both in the same jar to load them via reflection
>
> 3# Proxy
> Use proxies to generate the FullTextSessionImpl and
> FullTextEntityManagerImpl, delegating to whatever methods are
> available at runtime.
>
> The first one is simple and conservative; the second one seems a good
> idea but I've had experience with similar mess and I'm not too eager
> to fall in it again, the third one is quite more complex but has
> potential to get rid of these problems in the longer term.
>
> For the longer term, we also have this class committed in ORM:
>
> https://github.com/hibernate/hibernate-orm/blob/8c95a6077a523c47482fbae14ab54b763fa73a23/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java
>
> It would have resolved this problem, but we can't use this approach yet as
>  a) it's not included in the older ORMs yet, including some which we
> should still support
>  b) I forgot to make one for EntityManager :-(
>
> Preferences?
>
> Sanne
>
>
> On 4 September 2013 12:25, Gunnar Morling  wrote:
> > One option would be to (temporarily) establish an abstraction layer in
> > HSEARCH, which provides a uniform way for accessing the concerned
> > functionality from ORM.
> >
> > You would then have two implementations of this layer, one based on 4.2
> and
> > one based on 4.3 (e.g. in form of two separate HSEARCH modules built
> against
> > the different ORM versions). At runtime the right implementation would be
> > chosen depending on the ORM version present.
> >
> > I guess that would require some more coding on the HSEARCH side but IMO
> it
> > would be the cleanest approach for addressing the issue.
> >
> > --Gunnar
> >
> >
> >
> > 2013/9/4 Sanne Grinovero 
> >>
> >> There are some small changes in ORM 4.3 which are making HSearch
> >> incompatible, while I'm trying to keep Search compatible with bith ORM
> >> 4.2.x and 4.3 series.
> >>
> >> It's more complex than expected, so here is an update on what I'm doing:
> >>
> >> # Shadowing of interfaces being moved to new packages
> >>
> >> One change in particular is the move of certain Service interfaces to
> >> a new package location, we discussed on IRC that a possible solution
> >> would be to re-introduce a "shadow" interface: a patch to ORM
> >> re-introducing the missing interfaces as deprecated extensions of the
> >> new locations, so that projects compiling againts ORM 4.2 would still
> >> work (not guaranteed as I'm exclusively shadowing the ones that I'm
> >> needing for Search).
> >>
> >> In practice this solution doesn't work: we use these interface also as
> >> key for a lookup in the ServiceRegistry; obviously if the key is not
> >> the same class definition we fail to retrieve the service.
> >>
> >> At this point I tried to patch the ServiceRegistry itself to make it
> >> handle specifically the backwards-friendly shadow interfaces, but it
> >> looks like I can't do that as the method signature is locked-down:
> >>
> >> public  R getService(Class serviceRole) {
> >>   return (R) patchedGetService(); <---!!!
> >> }
> >>
> >> At this point I have to introduce the explicit casting or it won't
> >> compile, but then also the cast will fail at runtime unless I revert
> >> the paren

Re: [hibernate-dev] Hibernate Search compatibility with ORM 4.3

2013-09-05 Thread Sanne Grinovero
On 5 September 2013 08:27, Gunnar Morling  wrote:
> I'm not yet understanding #1. When compiling against 4.2, wouldn't the new
> methods be missing or, if you added them, refer to types not being present
> with 4.2? How would this option exactly look?

Exactly, it would look lik as merging
https://github.com/Sanne/hibernate-search/tree/4.2compatOnly

It contains the initial commits from the open pull you've been
reviewing, just missing the dependency change to ORM 4.3.x and the new
methods needed to implement all methods defined on the super
interfaces.
[It's also missing some changes in other modules in test helpers but
that's not relevant]

Basically - since it includes all other changes in the /ORM module -
it would work fine with ORM 4.2 (as tested) and also with ORM 4.3 (as
tested when applying the missing changes), with the exception of it
throwing an exception when one of the missing methods is invoked
(which are new methods).

I'm not liking the exceptions detail of the story but provided it's
well documented it's a step forward.

Actually it's a requirement for each other solution too, so I'll send
this pull request already... it would resolve

   "HSEARCH-1386 - NoClassDefFoundError when using Hibernate ORM 4.3.0"

But it would not resolve

   "HSEARCH-1373 - Update Hibernate ORM to 4.3"

yet.


> For the adapters to work in this particular case, I think you'd have to move
> FTSI and FTEMI to the adapter modules which does not seem so nice, given
> that they are such central classes.
>
> So I think I'd go for the proxy approach. As you say we could get rid of the
> proxies once we depend on an ORM version which has this
> delegating/forwarding base class. And in the long-long term we might even
> benefit from Java 8 default methods for this kind of issue :)

+1, I'll polish the existing pull for integration of the first step
and start experimenting with JDK proxies.

Sanne

>
> --Gunnar
>
>
>
> 2013/9/4 Sanne Grinovero 
>>
>> That's one possibility, but to recap all options discussed so far:
>>
>> One alternative which I consider quite tempting is that we don't
>> implement the new methods defined in Session and EntityManager (JPA
>> 2.1), but keeping it fully compatible in all other ways.
>>
>> We have this option because I resolved all remaining issues and could
>> run the testsuite succesfully on both ORM 4.2 and 4.3, with these
>> exceptions:
>>  - needs to be compiled with ORM 4.3 to have access to the new types
>> used in the new method signatures
>>  - when an application is deployed on JBoss EAP 6.1 or 6.0 it fails
>> hard as it eagerly realizes that some types aren't availabled
>>
>> In other words that means that Hibernate Search 4.4 users would
>> benefit from improvements in latest ORM but not from the new JPA
>> features.
>>
>> Our options:
>>
>> 1# Support ORM 4.2
>> If we compile against ORM 4.2 with these patches, it will work on ORM
>> 4.3 as well, as long as the new methods aren't used.
>>
>> 2# Gunnar's adapters
>> have two separate Maven modules containing the two different adaptors
>> needed, and bundle both in the same jar to load them via reflection
>>
>> 3# Proxy
>> Use proxies to generate the FullTextSessionImpl and
>> FullTextEntityManagerImpl, delegating to whatever methods are
>> available at runtime.
>>
>> The first one is simple and conservative; the second one seems a good
>> idea but I've had experience with similar mess and I'm not too eager
>> to fall in it again, the third one is quite more complex but has
>> potential to get rid of these problems in the longer term.
>>
>> For the longer term, we also have this class committed in ORM:
>>
>> https://github.com/hibernate/hibernate-orm/blob/8c95a6077a523c47482fbae14ab54b763fa73a23/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java
>>
>> It would have resolved this problem, but we can't use this approach yet as
>>  a) it's not included in the older ORMs yet, including some which we
>> should still support
>>  b) I forgot to make one for EntityManager :-(
>>
>> Preferences?
>>
>> Sanne
>>
>>
>> On 4 September 2013 12:25, Gunnar Morling  wrote:
>> > One option would be to (temporarily) establish an abstraction layer in
>> > HSEARCH, which provides a uniform way for accessing the concerned
>> > functionality from ORM.
>> >
>> > You would then have two implementations of this layer, one based on 4.2
>> > and
>> > one based on 4.3 (e.g. in form of two separate HSEARCH modules built
>> > against
>> > the different ORM versions). At runtime the right implementation would
>> > be
>> > chosen depending on the ORM version present.
>> >
>> > I guess that would require some more coding on the HSEARCH side but IMO
>> > it
>> > would be the cleanest approach for addressing the issue.
>> >
>> > --Gunnar
>> >
>> >
>> >
>> > 2013/9/4 Sanne Grinovero 
>> >>
>> >> There are some small changes in ORM 4.3 which are making HSearch
>> >> incompatible, while I'm trying to keep Search compatible with bith OR

Re: [hibernate-dev] Feature for handling getNextValue work in same transaction.

2013-09-05 Thread Jeremy Whiting
Hi Steve,
  Since my last response I have gone back to the drawing board to 
re-evaluate and test the proposed change. In brief the change does 
inline the sql in the current transaction and uses pessimistic locking. 
This is a change to the initial ideas suggested. Reasons for that are 
later in the response.

  Comments in-line

On 12/08/13 02:51, Steve Ebersole wrote:
> First, you lump sequences and table-based sequence together here, but I
> assure you sequences (real database sequences) are read inline with the
> current transaction.  In the case of a real sequence, the database
> already handles the isolation of the generated values outside
> transactional context.  When you ask the database sequence for its next
> value, that value is incremented regardless of the outcome of the
> transaction.
>
> That is to say, your example "sequence of calls" is indicative of table
> based generation, not at all true of sequence based generation.  For the
> table based generation case, I am leery of doing this for a few
> reasons.  First, as Sanne points out, our "sequence table" strategy is
> purely meant to mimic a database sequence in terms of semantic in cases
> where a sequence cannot be used.
Yes table based generation.
> Secondly, you request this (I believe) as a performance enhancement as a
> means to work around your particular use-case which I think we can all
> agree is *at best* a questionable real-life scenario.  The problem with
> the performance aspect is that you are trading one type of performance
> concern (resource allocation) for another (blocking).  That is to say,
> currently you have a performance problem because accessing the database
> value causes multiple discrete transactions.  But what you suggest
> instead is to trade off the isolation with blocking *in the same
> frequency*.  To illustrate, lets take a (simplified) look at 2
> transactions attempting to generate an identifier from the same
> "sequence table":
>
> T1 - begin
> T2 - begin
> T1 - select val from seq_table for update
> T2 - select val from seq_table for update
> T1 - update seq_table set val = ? where val = ?
> T2 - update seq_table set val = ? where val = ?
> T1 - insert into my_entity ...
> T2 - insert into my_entity ...
> T1 - commit
> T2 - commit
>
> We can notice a few different things here.  In the *best* case scenario,
> T1 simply blocks T2 meaning that T1 and T2 are no longer concurrent.  T1
> must completely finish before T2 will be allowed to proceed; T2 will
> block either on the `select for update` or on the `update`,
> depending...  Yes, using separate transactions for reading the "sequence
> table" value still does blocking, the difference is one of scope and
> therefore duration.
  Yes I agree exactly this is what will happen. The tradeoff relies on 
the blocking of concurrent threads till T1 terminates.
  Testing of the application I am using has shown an improvement in 
response times. 5 graphs are shown. Scale excluded. The blue orange and 
yellow (1st three) bars show results using an isolated transaction and 
the green, brown and light blue (last three) show the inlined sql. See 
the attached pdf to the HHH-8429
https://hibernate.atlassian.net/browse/HHH-8429

  It shows the response time is improved most of the time or is equal. 
Shorter bars is better btw.

> Seems to me, if I were you, I'd be more interested in minimizing the
> number of times the database is called regardless of what happens when
> the database is called...
  I think by inlining the sql the system reduces the number of calls by 
three. To fetch the next block of ids. 3 being to manage the isolated 
concurrent transaction.

  The proposed idea for using optimistic locking was proved in testing 
to be too buggy. Scott's question in the jira demonstrates the issue. In 
my testing concurrent transactions did end up competing.

> On 08/11/2013 06:45 AM, Sanne Grinovero wrote:
>> On 9 August 2013 19:08, Jeremy Whiting  wrote:
>>> Hi Scott,
>>>To the database the sequence of statements will be as follows. The
>>> statements around the read and write of sequence table are an example to
>>> put the sequence_table work into context.
>>>
>>> tx1   BEGIN
>>> SELECT  blah
>>> SELECT blah
>>> UPDATE blah
>>> SELECT * from sequence_table;
>>> UPDATE sequence_table SET ids=?;
>>> INSERT INTO blah
>>> UPDATE blah
>>> tx1 END
>>> tx1 COMMIT
>>>
>>>The work will not be isolated from the current transaction tx1.
>>> Concurrently running transactions will see the changes to sequence_table
>>> depending on the isolation level set when the pool is filled.
>> What is the use case for creating the sequence in the same transaction?
>> Sequences are usually needed to be absolutely independent from transactions.
>> That is generally the case for "real" sequence constructs:
>>- 
>> http://docs.oracle.com/cd/B28359_01/server.111/b28286/statements_6015.htm
>>
>> With sequence tables we're providing alternative strategies but -as a

Re: [hibernate-dev] hiberate-commons-annotations and locating annotations

2013-09-05 Thread Steve Ebersole
I really need someone who actually understands the AnnotationBinder and 
commons-annotations stuff to look at this and help me find the plan for 
applying attribute converters.

 From what I can tell, PropertyHolder is the proper place to perform the 
resolution of which converter to use because that allows for the 
variance in how they resolution is handled in the 
class/composite/collection cases.  But PropertyHolder by itself does 
not have all the info it needs, specifically access to the XProperty 
objects.  And my simple attempt to have PropertyBinder tell 
PropertyHolder about the XProperty "works", but pretty sure it breaks 
down in the composite path cases due to the ordering of the calls 
(really need the inner most converts applied first).

As of now, 4.3 Beta4 is hung up on getting this sorted out.


On Wed 04 Sep 2013 11:43:19 PM CDT, Steve Ebersole wrote:
> Stepping through the code showed me that PropertyHolder#addProperty
> will not be useful here. Its called after the corresponding
> SimpleValueBinder#setType call. Really not understanding how these are
> intended to work.
>
> I have some of the normalization working, hooked into PropertyHolder
> constructors. But if this is the correct approach, I am not following
> how to do a few things.
>
> For example, in ClassPropertyHolder I have this:
> https://gist.github.com/sebersole/6446095  Essentially it traverses
> the super classes of the entity (depth first) and adds conversions it
> finds. That part is fine. The trouble is outlined at the end of that
> method in the comment. The situation is this:
>
> @MappedSuperclass
> public static class Super {
> ...
> @Convert(converter = SillyStringConverter.class)
> public String it;
> }
>
> @Entity(name = "Sub")
> @Convert( attributeName = "it", disableConversion = true )
> public static class Sub extends Super {
> }
>
> In building the "normalized" map of conversions for Sub, I really need
> to be able to look at its attributes.  But I am unsure how to do that
> given just the XClass.  There is the XClass#getDeclaredProperties, but
> to use that I need to know about field/property access.  I don't think
> we know about field/property access at that point in ClassPropertyHolder.
>
> Also, I am not sure that iterating properties yet again is a great idea.
>
> One alternative I had considered was to hook this in with
> PropertyBinder, where it calls SimpleValueBinder.  That needs to
> happen anyway; the plan was to delegate the call to determine the
> Convert to use PropertyHolder and to pass that Convert into
> SimpleValueBinder. That code would need to pass in the XProperty
> anyway.  So currently PropertyBinder does:
>
> simpleValueBinder.setType( property, returnedClass, containerClassName );
>
> My plan was to instead have it do:
>
> simpleValueBinder.setType( property, returnedClass,
> containerClassName, holder.resolveAttributeConverter( property ) );
>
> I *think* that when resolveAttributeConverter() would be called, we'd
> have enough info to fully resolve the converter to use properly.
>
> There are similar concerns in ComponentPropertyHolder, but maybe a
> discussion of the above will shed light on those concerns too.
>
>
>
> On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote:
>>
>> I am still a bit confused on how to apply the normalization to make
>> sure it happens in the proper order.
>>
>> Let's look at:
>>
>> @Entity
>> class Person {
>> ...
>> @Embedded
>> @Convert( attributeName="city", converter=Converter2.class )
>> public Address homeAddress;
>> }
>>
>> @Embeddable
>> class Address {
>> ...
>> @Convert( converter=Converter1.class )
>> public String city;
>> }
>>
>>
>> So here, the Converter2 class ought to be the one applied to
>> "homeAddress.city" path.
>>
>> Now granted there are a few different ways to skin this cat, but the
>> plan I had was to normalize these all on the root of the path. So
>> here, both of these conversions would get stored on
>> ClassPropertyHolder under the "homeAddress.city" path. When I
>> go to build the SimpleValue for Person.homeAddress.city, I'd ask the
>> CompositePropertyHolder for Person.homeAddress to resolve the explicit
>> (non-autoApply) conversion for its city attribute (the simple value).
>> The trouble I have though is applying the conversions in the proper
>> order. For example, here, I'd want to apply the conversions defined
>> directly on the Embeddable first (the Embeddable conversions should
>> always act as the baseline), then the conversions at its Embedded
>> point (via XML or annotations).
>>
>> One idea was to hook into org.hibernate.cfg.PropertyHolder#addProperty
>> in terms of normalizing the paths. I am just not sure of the timing
>> between these PropertyHolder#addProperty (and how populated the passed
>> Property objects are at that point) and the calls to
>> SimpleValueBinder/PropertyBinder.
>>
>> Interestingly, I still am not sure we have enough here to report an
>> error in cases like:
>>
>> @Entity

[hibernate-dev] IRC Developer Meeting - Sept 5

2013-09-05 Thread Steve Ebersole
Discussed finishing up AttributeConverter resolution, LoadPlan progress 
and some JPA TCK points.

[10:46]  Minutes: 
http://transcripts.jboss.org/meeting/irc.freenode.org/hibernate-dev/2013/hibernate-dev.2013-09-05-15.03.html
[10:46]  Minutes (text): 
http://transcripts.jboss.org/meeting/irc.freenode.org/hibernate-dev/2013/hibernate-dev.2013-09-05-15.03.txt
[10:46]  Log: 
http://transcripts.jboss.org/meeting/irc.freenode.org/hibernate-dev/2013/hibernate-dev.2013-09-05-15.03.log.html
 

___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev


Re: [hibernate-dev] hiberate-commons-annotations and locating annotations

2013-09-05 Thread Steve Ebersole
On Thu 05 Sep 2013 11:52:22 AM CDT, Shaozhuang Liu wrote:
>> Also, I am not sure that iterating properties yet again is a great idea.
>>
>> One alternative I had considered was to hook this in with
>> PropertyBinder, where it calls SimpleValueBinder.  That needs to happen
>> anyway; the plan was to delegate the call to determine the Convert to
>> use PropertyHolder and to pass that Convert into SimpleValueBinder.
>> That code would need to pass in the XProperty anyway.  So currently
>> PropertyBinder does:
>>
>> simpleValueBinder.setType( property, returnedClass, containerClassName );
>>
>> My plan was to instead have it do:
>>
>> simpleValueBinder.setType( property, returnedClass, containerClassName,
>> holder.resolveAttributeConverter( property ) );
>>
>> I *think* that when resolveAttributeConverter() would be called, we'd
>> have enough info to fully resolve the converter to use properly.
>>
>> There are similar concerns in ComponentPropertyHolder, but maybe a
>> discussion of the above will shed light on those concerns too.
>
> doesn't this work?

What exactly?


> at this time, the PropertyHolder is resolved, so it knows the convert it 
> holds,
> then the convert defined on the attribute get applied first, then the one in 
> PropertyHolder

I am not sure what you mean by "resolved" here.  At no time that is 
useful in this process does PropertyHolder have full access to all the 
properties the thing holds.

If by "the convert it holds" you mean the converts specifically defined 
on the @Entity or @Embedded (and therefore @Embeddable), then yes.

But "then the convert defined on the attribute get applied first" is 
actually wrong for composite paths.  For composites, you actually want 
the one higher up to have higher precedence.  In the 
Person.homeAddress.city example, a convert defined on Address#city 
should actually have the *lowest* precedence.

I think what we'll have to end up doing is to not normalize the 
composite paths to one place unfortunately.   Then we'd have to pass 
the XProperty for which we are trying to resolve the converter to use 
into the PropertyHolder method.  For composite paths this will just 
have to mean walking multiple levels (one per path-part).  The down 
side to this is that at no time do we have an overview of the overall 
converts for a property path (aside from walking the composite path to 
actually resolve the converter to use).


>
> any problem with this approach ?
>
>>
>>
>>
>> On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote:
>>>
>>> I am still a bit confused on how to apply the normalization to make
>>> sure it happens in the proper order.
>>>
>>> Let's look at:
>>>
>>> @Entity
>>> class Person {
>>> ...
>>> @Embedded
>>> @Convert( attributeName="city", converter=Converter2.class )
>>> public Address homeAddress;
>>> }
>>>
>>> @Embeddable
>>> class Address {
>>> ...
>>> @Convert( converter=Converter1.class )
>>> public String city;
>>> }
>>>
>>>
>>> So here, the Converter2 class ought to be the one applied to
>>> "homeAddress.city" path.
>>>
>>> Now granted there are a few different ways to skin this cat, but the
>>> plan I had was to normalize these all on the root of the path. So
>>> here, both of these conversions would get stored on
>>> ClassPropertyHolder under the "homeAddress.city" path. When I
>>> go to build the SimpleValue for Person.homeAddress.city, I'd ask the
>>> CompositePropertyHolder for Person.homeAddress to resolve the explicit
>>> (non-autoApply) conversion for its city attribute (the simple value).
>>> The trouble I have though is applying the conversions in the proper
>>> order. For example, here, I'd want to apply the conversions defined
>>> directly on the Embeddable first (the Embeddable conversions should
>>> always act as the baseline), then the conversions at its Embedded
>>> point (via XML or annotations).
>>>
>>> One idea was to hook into org.hibernate.cfg.PropertyHolder#addProperty
>>> in terms of normalizing the paths. I am just not sure of the timing
>>> between these PropertyHolder#addProperty (and how populated the passed
>>> Property objects are at that point) and the calls to
>>> SimpleValueBinder/PropertyBinder.
>>>
>>> Interestingly, I still am not sure we have enough here to report an
>>> error in cases like:
>>>
>>> @Entity
>>> @Convert( attributeName="homeAddress.city", converter=Converter3.class )
>>> class Person {
>>> ...
>>> @Embedded
>>> @Convert( attributeName="city", converter=Converter2.class )
>>> public Address homeAddress;
>>> }
>>>
>>> Unless we somehow kept "proximity info" or "location info" about the
>>> conversion in PropertyHolder.
>>>
>>>
>>> On Wed 04 Sep 2013 01:27:21 AM CDT, Emmanuel Bernard wrote:


 On Tue 2013-09-03 17:22, Steve Ebersole wrote:
>
>
> 2.a) It seems like there are times when
> org.hibernate.cfg.AbstractPropertyHolder#parent would be useful for
> what I need to do. But there appears to be times when this is null.
> For entity mappings

Re: [hibernate-dev] hiberate-commons-annotations and locating annotations

2013-09-05 Thread Shaozhuang Liu


On 2013Sep 5, at 12:43 PM, Steve Ebersole  wrote:

> Stepping through the code showed me that PropertyHolder#addProperty will 
> not be useful here. Its called after the corresponding 
> SimpleValueBinder#setType call. Really not understanding how these are 
> intended to work.
> 
> I have some of the normalization working, hooked into PropertyHolder 
> constructors. But if this is the correct approach, I am not following 
> how to do a few things.
> 
> For example, in ClassPropertyHolder I have this: 
> https://gist.github.com/sebersole/6446095  Essentially it traverses the 
> super classes of the entity (depth first) and adds conversions it finds. 
> That part is fine. The trouble is outlined at the end of that method in 
> the comment. The situation is this:
> 
> @MappedSuperclass
> public static class Super {
> ...
> @Convert(converter = SillyStringConverter.class)
> public String it;
> }
> 
> @Entity(name = "Sub")
> @Convert( attributeName = "it", disableConversion = true )
> public static class Sub extends Super {
> }
> 
> In building the "normalized" map of conversions for Sub, I really need 
> to be able to look at its attributes.  But I am unsure how to do that 
> given just the XClass.  There is the XClass#getDeclaredProperties, but 
> to use that I need to know about field/property access.  I don't think 
> we know about field/property access at that point in ClassPropertyHolder.

no, we don't know about this I think, and also, field/property can have 
explicit defined access type

> 
> Also, I am not sure that iterating properties yet again is a great idea.
> 
> One alternative I had considered was to hook this in with 
> PropertyBinder, where it calls SimpleValueBinder.  That needs to happen 
> anyway; the plan was to delegate the call to determine the Convert to 
> use PropertyHolder and to pass that Convert into SimpleValueBinder.  
> That code would need to pass in the XProperty anyway.  So currently 
> PropertyBinder does:
> 
> simpleValueBinder.setType( property, returnedClass, containerClassName );
> 
> My plan was to instead have it do:
> 
> simpleValueBinder.setType( property, returnedClass, containerClassName, 
> holder.resolveAttributeConverter( property ) );
> 
> I *think* that when resolveAttributeConverter() would be called, we'd 
> have enough info to fully resolve the converter to use properly.
> 
> There are similar concerns in ComponentPropertyHolder, but maybe a 
> discussion of the above will shed light on those concerns too.

doesn't this work?
at this time, the PropertyHolder is resolved, so it knows the convert it holds, 
then the convert defined on the attribute get applied first, then the one in 
PropertyHolder

any problem with this approach ?

> 
> 
> 
> On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote:
>> 
>> I am still a bit confused on how to apply the normalization to make
>> sure it happens in the proper order.
>> 
>> Let's look at:
>> 
>> @Entity
>> class Person {
>> ...
>> @Embedded
>> @Convert( attributeName="city", converter=Converter2.class )
>> public Address homeAddress;
>> }
>> 
>> @Embeddable
>> class Address {
>> ...
>> @Convert( converter=Converter1.class )
>> public String city;
>> }
>> 
>> 
>> So here, the Converter2 class ought to be the one applied to
>> "homeAddress.city" path.
>> 
>> Now granted there are a few different ways to skin this cat, but the
>> plan I had was to normalize these all on the root of the path. So
>> here, both of these conversions would get stored on
>> ClassPropertyHolder under the "homeAddress.city" path. When I
>> go to build the SimpleValue for Person.homeAddress.city, I'd ask the
>> CompositePropertyHolder for Person.homeAddress to resolve the explicit
>> (non-autoApply) conversion for its city attribute (the simple value).
>> The trouble I have though is applying the conversions in the proper
>> order. For example, here, I'd want to apply the conversions defined
>> directly on the Embeddable first (the Embeddable conversions should
>> always act as the baseline), then the conversions at its Embedded
>> point (via XML or annotations).
>> 
>> One idea was to hook into org.hibernate.cfg.PropertyHolder#addProperty
>> in terms of normalizing the paths. I am just not sure of the timing
>> between these PropertyHolder#addProperty (and how populated the passed
>> Property objects are at that point) and the calls to
>> SimpleValueBinder/PropertyBinder.
>> 
>> Interestingly, I still am not sure we have enough here to report an
>> error in cases like:
>> 
>> @Entity
>> @Convert( attributeName="homeAddress.city", converter=Converter3.class )
>> class Person {
>> ...
>> @Embedded
>> @Convert( attributeName="city", converter=Converter2.class )
>> public Address homeAddress;
>> }
>> 
>> Unless we somehow kept "proximity info" or "location info" about the
>> conversion in PropertyHolder.
>> 
>> 
>> On Wed 04 Sep 2013 01:27:21 AM CDT, Emmanuel Bernard wrote:
>>> 
>>> 
>>> On Tue 2013-09-03 17:22, Steve Ebersole w

Re: [hibernate-dev] hiberate-commons-annotations and locating annotations

2013-09-05 Thread Steve Ebersole
I have something mostly working and not causing regressions.  Its quite 
fugly, but I blame that on binders and hcann :)

I'd really like to do a pull request for this one and have y'all take a 
look.  But at the same time I'd also really like to do the 4.3 Beta4 
release tomorrow.  Anyone familiar with those bits of code willing to 
look this over in the next 12 hours if I go through the steps of 
creating the PR?


On 09/05/2013 12:10 PM, Steve Ebersole wrote:
> On Thu 05 Sep 2013 11:52:22 AM CDT, Shaozhuang Liu wrote:
>>> Also, I am not sure that iterating properties yet again is a great 
>>> idea.
>>>
>>> One alternative I had considered was to hook this in with
>>> PropertyBinder, where it calls SimpleValueBinder.  That needs to happen
>>> anyway; the plan was to delegate the call to determine the Convert to
>>> use PropertyHolder and to pass that Convert into SimpleValueBinder.
>>> That code would need to pass in the XProperty anyway.  So currently
>>> PropertyBinder does:
>>>
>>> simpleValueBinder.setType( property, returnedClass, 
>>> containerClassName );
>>>
>>> My plan was to instead have it do:
>>>
>>> simpleValueBinder.setType( property, returnedClass, containerClassName,
>>> holder.resolveAttributeConverter( property ) );
>>>
>>> I *think* that when resolveAttributeConverter() would be called, we'd
>>> have enough info to fully resolve the converter to use properly.
>>>
>>> There are similar concerns in ComponentPropertyHolder, but maybe a
>>> discussion of the above will shed light on those concerns too.
>>
>> doesn't this work?
>
> What exactly?
>
>
>> at this time, the PropertyHolder is resolved, so it knows the convert 
>> it holds,
>> then the convert defined on the attribute get applied first, then the 
>> one in PropertyHolder
>
> I am not sure what you mean by "resolved" here.  At no time that is 
> useful in this process does PropertyHolder have full access to all the 
> properties the thing holds.
>
> If by "the convert it holds" you mean the converts specifically 
> defined on the @Entity or @Embedded (and therefore @Embeddable), then 
> yes.
>
> But "then the convert defined on the attribute get applied first" is 
> actually wrong for composite paths.  For composites, you actually want 
> the one higher up to have higher precedence.  In the 
> Person.homeAddress.city example, a convert defined on Address#city 
> should actually have the *lowest* precedence.
>
> I think what we'll have to end up doing is to not normalize the 
> composite paths to one place unfortunately.   Then we'd have to pass 
> the XProperty for which we are trying to resolve the converter to use 
> into the PropertyHolder method.  For composite paths this will just 
> have to mean walking multiple levels (one per path-part).  The down 
> side to this is that at no time do we have an overview of the overall 
> converts for a property path (aside from walking the composite path to 
> actually resolve the converter to use).
>
>
>>
>> any problem with this approach ?
>>
>>>
>>>
>>>
>>> On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote:

 I am still a bit confused on how to apply the normalization to make
 sure it happens in the proper order.

 Let's look at:

 @Entity
 class Person {
 ...
 @Embedded
 @Convert( attributeName="city", converter=Converter2.class )
 public Address homeAddress;
 }

 @Embeddable
 class Address {
 ...
 @Convert( converter=Converter1.class )
 public String city;
 }


 So here, the Converter2 class ought to be the one applied to
 "homeAddress.city" path.

 Now granted there are a few different ways to skin this cat, but the
 plan I had was to normalize these all on the root of the path. So
 here, both of these conversions would get stored on
 ClassPropertyHolder under the "homeAddress.city" path. When I
 go to build the SimpleValue for Person.homeAddress.city, I'd ask the
 CompositePropertyHolder for Person.homeAddress to resolve the explicit
 (non-autoApply) conversion for its city attribute (the simple value).
 The trouble I have though is applying the conversions in the proper
 order. For example, here, I'd want to apply the conversions defined
 directly on the Embeddable first (the Embeddable conversions should
 always act as the baseline), then the conversions at its Embedded
 point (via XML or annotations).

 One idea was to hook into org.hibernate.cfg.PropertyHolder#addProperty
 in terms of normalizing the paths. I am just not sure of the timing
 between these PropertyHolder#addProperty (and how populated the passed
 Property objects are at that point) and the calls to
 SimpleValueBinder/PropertyBinder.

 Interestingly, I still am not sure we have enough here to report an
 error in cases like:

 @Entity
 @Convert( attributeName="homeAddress.city", 
 converter=Converter3.class )
>>

Re: [hibernate-dev] hiberate-commons-annotations and locating annotations

2013-09-05 Thread Shaozhuang Liu
I can give it a try
-
Best Regards,

Strong Liu 
http://about.me/stliu/bio

On 2013Sep 6, at 8:15 AM, Steve Ebersole  wrote:

> I have something mostly working and not causing regressions.  Its quite 
> fugly, but I blame that on binders and hcann :)
> 
> I'd really like to do a pull request for this one and have y'all take a look. 
>  But at the same time I'd also really like to do the 4.3 Beta4 release 
> tomorrow.  Anyone familiar with those bits of code willing to look this over 
> in the next 12 hours if I go through the steps of creating the PR?
> 
> 
> On 09/05/2013 12:10 PM, Steve Ebersole wrote:
>> On Thu 05 Sep 2013 11:52:22 AM CDT, Shaozhuang Liu wrote:
 Also, I am not sure that iterating properties yet again is a great idea.
 
 One alternative I had considered was to hook this in with
 PropertyBinder, where it calls SimpleValueBinder.  That needs to happen
 anyway; the plan was to delegate the call to determine the Convert to
 use PropertyHolder and to pass that Convert into SimpleValueBinder.
 That code would need to pass in the XProperty anyway.  So currently
 PropertyBinder does:
 
 simpleValueBinder.setType( property, returnedClass, containerClassName );
 
 My plan was to instead have it do:
 
 simpleValueBinder.setType( property, returnedClass, containerClassName,
 holder.resolveAttributeConverter( property ) );
 
 I *think* that when resolveAttributeConverter() would be called, we'd
 have enough info to fully resolve the converter to use properly.
 
 There are similar concerns in ComponentPropertyHolder, but maybe a
 discussion of the above will shed light on those concerns too.
>>> 
>>> doesn't this work?
>> 
>> What exactly?
>> 
>> 
>>> at this time, the PropertyHolder is resolved, so it knows the convert it 
>>> holds,
>>> then the convert defined on the attribute get applied first, then the one 
>>> in PropertyHolder
>> 
>> I am not sure what you mean by "resolved" here.  At no time that is useful 
>> in this process does PropertyHolder have full access to all the properties 
>> the thing holds.
>> 
>> If by "the convert it holds" you mean the converts specifically defined on 
>> the @Entity or @Embedded (and therefore @Embeddable), then yes.
>> 
>> But "then the convert defined on the attribute get applied first" is 
>> actually wrong for composite paths.  For composites, you actually want the 
>> one higher up to have higher precedence.  In the Person.homeAddress.city 
>> example, a convert defined on Address#city should actually have the *lowest* 
>> precedence.
>> 
>> I think what we'll have to end up doing is to not normalize the composite 
>> paths to one place unfortunately.   Then we'd have to pass the XProperty for 
>> which we are trying to resolve the converter to use into the PropertyHolder 
>> method.  For composite paths this will just have to mean walking multiple 
>> levels (one per path-part).  The down side to this is that at no time do we 
>> have an overview of the overall converts for a property path (aside from 
>> walking the composite path to actually resolve the converter to use).
>> 
>> 
>>> 
>>> any problem with this approach ?
>>> 
 
 
 
 On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote:
> 
> I am still a bit confused on how to apply the normalization to make
> sure it happens in the proper order.
> 
> Let's look at:
> 
> @Entity
> class Person {
> ...
> @Embedded
> @Convert( attributeName="city", converter=Converter2.class )
> public Address homeAddress;
> }
> 
> @Embeddable
> class Address {
> ...
> @Convert( converter=Converter1.class )
> public String city;
> }
> 
> 
> So here, the Converter2 class ought to be the one applied to
> "homeAddress.city" path.
> 
> Now granted there are a few different ways to skin this cat, but the
> plan I had was to normalize these all on the root of the path. So
> here, both of these conversions would get stored on
> ClassPropertyHolder under the "homeAddress.city" path. When I
> go to build the SimpleValue for Person.homeAddress.city, I'd ask the
> CompositePropertyHolder for Person.homeAddress to resolve the explicit
> (non-autoApply) conversion for its city attribute (the simple value).
> The trouble I have though is applying the conversions in the proper
> order. For example, here, I'd want to apply the conversions defined
> directly on the Embeddable first (the Embeddable conversions should
> always act as the baseline), then the conversions at its Embedded
> point (via XML or annotations).
> 
> One idea was to hook into org.hibernate.cfg.PropertyHolder#addProperty
> in terms of normalizing the paths. I am just not sure of the timing
> between these PropertyHolder#addProperty (and how populated the passed
> Property objec

Re: [hibernate-dev] hiberate-commons-annotations and locating annotations

2013-09-05 Thread Steve Ebersole
https://github.com/hibernate/hibernate-orm/pull/591


On Thu 05 Sep 2013 10:06:14 PM CDT, Shaozhuang Liu wrote:
> I can give it a try
> -
> Best Regards,
>
> Strong Liu 
> http://about.me/stliu/bio
>
> On 2013Sep 6, at 8:15 AM, Steve Ebersole  wrote:
>
>> I have something mostly working and not causing regressions.  Its quite 
>> fugly, but I blame that on binders and hcann :)
>>
>> I'd really like to do a pull request for this one and have y'all take a 
>> look.  But at the same time I'd also really like to do the 4.3 Beta4 release 
>> tomorrow.  Anyone familiar with those bits of code willing to look this over 
>> in the next 12 hours if I go through the steps of creating the PR?
>>
>>
>> On 09/05/2013 12:10 PM, Steve Ebersole wrote:
>>> On Thu 05 Sep 2013 11:52:22 AM CDT, Shaozhuang Liu wrote:
> Also, I am not sure that iterating properties yet again is a great idea.
>
> One alternative I had considered was to hook this in with
> PropertyBinder, where it calls SimpleValueBinder.  That needs to happen
> anyway; the plan was to delegate the call to determine the Convert to
> use PropertyHolder and to pass that Convert into SimpleValueBinder.
> That code would need to pass in the XProperty anyway.  So currently
> PropertyBinder does:
>
> simpleValueBinder.setType( property, returnedClass, containerClassName );
>
> My plan was to instead have it do:
>
> simpleValueBinder.setType( property, returnedClass, containerClassName,
> holder.resolveAttributeConverter( property ) );
>
> I *think* that when resolveAttributeConverter() would be called, we'd
> have enough info to fully resolve the converter to use properly.
>
> There are similar concerns in ComponentPropertyHolder, but maybe a
> discussion of the above will shed light on those concerns too.

 doesn't this work?
>>>
>>> What exactly?
>>>
>>>
 at this time, the PropertyHolder is resolved, so it knows the convert it 
 holds,
 then the convert defined on the attribute get applied first, then the one 
 in PropertyHolder
>>>
>>> I am not sure what you mean by "resolved" here.  At no time that is useful 
>>> in this process does PropertyHolder have full access to all the properties 
>>> the thing holds.
>>>
>>> If by "the convert it holds" you mean the converts specifically defined on 
>>> the @Entity or @Embedded (and therefore @Embeddable), then yes.
>>>
>>> But "then the convert defined on the attribute get applied first" is 
>>> actually wrong for composite paths.  For composites, you actually want the 
>>> one higher up to have higher precedence.  In the Person.homeAddress.city 
>>> example, a convert defined on Address#city should actually have the 
>>> *lowest* precedence.
>>>
>>> I think what we'll have to end up doing is to not normalize the composite 
>>> paths to one place unfortunately.   Then we'd have to pass the XProperty 
>>> for which we are trying to resolve the converter to use into the 
>>> PropertyHolder method.  For composite paths this will just have to mean 
>>> walking multiple levels (one per path-part).  The down side to this is that 
>>> at no time do we have an overview of the overall converts for a property 
>>> path (aside from walking the composite path to actually resolve the 
>>> converter to use).
>>>
>>>

 any problem with this approach ?

>
>
>
> On Wed 04 Sep 2013 02:44:40 PM CDT, Steve Ebersole wrote:
>>
>> I am still a bit confused on how to apply the normalization to make
>> sure it happens in the proper order.
>>
>> Let's look at:
>>
>> @Entity
>> class Person {
>> ...
>> @Embedded
>> @Convert( attributeName="city", converter=Converter2.class )
>> public Address homeAddress;
>> }
>>
>> @Embeddable
>> class Address {
>> ...
>> @Convert( converter=Converter1.class )
>> public String city;
>> }
>>
>>
>> So here, the Converter2 class ought to be the one applied to
>> "homeAddress.city" path.
>>
>> Now granted there are a few different ways to skin this cat, but the
>> plan I had was to normalize these all on the root of the path. So
>> here, both of these conversions would get stored on
>> ClassPropertyHolder under the "homeAddress.city" path. When I
>> go to build the SimpleValue for Person.homeAddress.city, I'd ask the
>> CompositePropertyHolder for Person.homeAddress to resolve the explicit
>> (non-autoApply) conversion for its city attribute (the simple value).
>> The trouble I have though is applying the conversions in the proper
>> order. For example, here, I'd want to apply the conversions defined
>> directly on the Embeddable first (the Embeddable conversions should
>> always act as the baseline), then the conversions at its Embedded
>> point (via XML or annotations).
>>
>> One idea was to hook into org.h