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.anton...@gmail.com > <mailto:lukasz.anton...@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 <st...@hibernate.org > <mailto:st...@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! > > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
