[hibernate-dev] Envers compile issue

[Steve Ebersole]

Hi Lukasz,

The problems seem limited to usages of:
1) 
org.hibernate.envers.configuration.internal.metadata.MetadataTools#getColumnNameIterator
2) 
org.hibernate.envers.configuration.internal.metadata.MetadataTools#addColumns

from:

1) 
org.hibernate.envers.configuration.internal.metadata.AuditMetadataGenerator#createJoins
2) 
org.hibernate.envers.configuration.internal.metadata.BasicMetadataGenerator#addManyToOne
3) 
org.hibernate.envers.configuration.internal.metadata.BasicMetadataGenerator#addBasic
4) 
org.hibernate.envers.configuration.internal.metadata.CollectionMetadataGenerator#addWithMiddleTable
5) 
org.hibernate.envers.configuration.internal.metadata.CollectionMetadataGenerator#addValueToMiddleTable
6) 
org.hibernate.envers.configuration.internal.metadata.ToOneRelationMetadataGenerator#addToOne


org.hibernate.envers.configuration.internal.metadata.MetadataTools#addColumns 
unfortunately goes through a number of internal addColumns/addColumn calls.

Oddly enough, I found that there is in fact:
1) 
org.hibernate.envers.configuration.internal.metadata.MetadataTools#addColumnsOrFormulas
2) 
org.hibernate.envers.configuration.internal.metadata.MetadataTools#addFormula

But that is only ever used to handle discriminators, as you mentioned.  
Additionally, I then text searched the envers testsuite and found that 
only the 
org.hibernate.envers.test.integration.inheritance.single.discriminatorformula 
package contained any references to the text 'formula', which further 
illustrates that.

So, all that said, I *think* envers already errored when it encountered 
formula (with CCE).  In the code I saw it is clearly expecting 
Iterator<Column> and not even checking if the Iterator returned Formula 
elements mixed in.

At the end of the day it is still y'alls call.  Based on my reading of 
the current code, I think a comparable (though better) solution is to 
explicitly check each Iterator element type and throw that more 
descriptive exception[1].  That change I can make.  If y'all would 
rather add in real formula support, thats probably something y'all will 
have to work on.

WDYT?


[1] For example, FormulaNotSupportException( "Formula mappings are 
currently not supported in envers aside from @DiscriminatorValue; for 
more information, please see blah-blah-blah..." )

On Apr 12, 2013 4:52 AM, "Łukasz Antoniak" <lukasz.antoniak at gmail.com 
<mailto:lukasz.antoniak at gmail.com>> wrote:

    Hello Steve,

    In general Envers does not support formula columns. Probably we
    should save formula value to audit table. This can be tricky and I
    have to think it over. Copying formula column to audit entity would
    not produce expected result in all cases (e.g. time functions used
    in formula expression). Currently we support @DiscriminatorFormula
    annotation without saving its value to audit table, since entities
    do not change their type.

    @Adam: Thoughts? What do you think about copying formula column to
    audit entity directly? Simplest solution AFAIK.

    All usages of org.hibernate.mapping.Value#getColumnIterator(),
    except the one in
    org.hibernate.envers.configuration.internal.metadata.BasicMetadataGenerator,
    map relation properties. IMO it is safe assumption that we are
    dealing with columns there.

    @Steve: if you agree, feel free to throw an exception if encountered
    formula column in BasicMetadataGenerator asking to register new feature.

    Best regards,
    Lukasz



    2013/4/11 Steve Ebersole <steve at hibernate.org
    <mailto:steve at hibernate.org>>

        One option just came to mind.  Looking at the code, I have to
        assume envers really just does not support formulas.  Otherwise,
        all these compile errors would eventually have resulted in
        runtime (CCE) exceptions.

        If that is really the case, I could add detection of that and
        throw a more meaningful exception.  If that sounds like a
        reasonable option, just let me know and I'll make that change.

        Alternatively I could just log a message and keep going, but i
        don't think that's a good choice.



        On Thu 11 Apr 2013 04:07:27 PM CDT, Steve Ebersole wrote:

            I just made a change to help catch runtime problems that
            kept cropping
            up.  The change was to
            org.hibernate.mapping.Value#getColumnIterator.
            The problem is that code in many modules (hem, envers) that
            actually
            deals with mapping code were making a bad assumption here.  The
            returned iterator actually returns a Iterable<Selectable>, not
            Iterable<Column>. Selectable is the contract shared between
            Column and
            Formula.  So when code non-defensively tries to treat that
            thing as a
            Iterable<Column> we often have issues.

            Yes, the method is very poorly named.  Actually it predates
            formula as
            a feature.  But be that as it may, the code casting those
            elements to
            Column are just wrong.

            For envers, I am not actually sure how to handle Formula
            elements. I
            need some help there.  The issues are all isolated to
            org.hibernate.envers.configuration.internal.metadata  Could
            someone
            more familiar with envers (and especially its entity
            definition stuff)
            take a look?

            Thanks!