In the meantime (to get the builds working again) I went ahead and
implemented the explicit checking. If you want to change something
there, just look for usages of the new
org.hibernate.envers.configuration.internal.metadata.FormulaNotSupportedException.
There are only 2 places it is used, both in MetadataTools...
On Fri 12 Apr 2013 08:03:59 AM CDT, Steve Ebersole wrote:
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(a)gmail.com
<mailto:lukasz.antoniak@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(a)hibernate.org
<mailto:steve@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!