Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [eclipselink-dev] Question aboutEmbeddedAccessor#processEmbeddableClass()

Hi Guy!

Sorry to disturb you again, but I still have a few problems ;)

I tried the way you suggested (had a similar construct before), and the mapping works fine. 
The problem which still exists concerns the fieldTranslation (AggregateObjectMapping#translateFields): 

While recursing into the the tree of the nested @Embedding elements, we will call addFieldNameTranslation for each layer, beginning with the embedding which is nearest to the leafe up to the root. So if we assume the following example (same as last time):

PriceWatch
 + id
 + TaxedPrice highestPrice (H_)
 |   + Price netPrice (N_)
 |   |   + BigDecimal amount
 |   |   + String currency
 |   |   
 |   + Price grossPrice (G_)
 |       + BigDecimal amount
 |       + String currency
 |
 + TaxedPrice lowestPrice (L_)
     + Price netPrice (N_)
     |   + BigDecimal amount
     |   + String currency
     |   
     + Price grossPrice (G_)
         + BigDecimal amount
         + String currency

We will call addFieldNameTranslation twice for the same field (first netPrice occurrence as example).
The 1st time while @Embedding highestPrice.netPrice is being processed.
The 2nd time while @Embedding highestPrice is being processed (by recursing down to netPrice.

So we will end up having 2 translations for the field:
.) N_AMOUNT = AMOUNT
.) H_N_AMOUNT = AMOUNT

The problem is: in AggregateObjectMapping#translateFields the processing seems to be 'in order', so after the code translages N_AMOUNT = AMOUNT, there is NO such field 'AMOUNT' anymore! Thus, the 2nd translation will simply fail. I tried to change the fieldTranslation H_N_AMOUNT = AMOUNT to H_N_AMOUNT = N_AMOUNT in the debugger, and got the correct result.

How can we work around it:
1.) as mentioned earlier: 'push' the prefix into the process() chain from top to bottom
2.) If I would know if the current embedding is nested in another embedding, I may omit the whole processing and only recurse down if this isn't the case (aka this is the 'outer most' embedding).

So, does the EmbeddableAccessor know if it is embedded too? The owningDescriptor always point to the 'real' Entity, so I may look if the owning descriptor does contain myself as field? But this is not very effective anyway ;) Any suggestions?

Btw: @AttributeOverride also calls addFieldNameTranslation, may this cause similar problems? How is this supposed to work if the inner and outer embedding contain different column name specs for the same field?

txs and LieGrue,
strub 

PS: If anyone is interrested in having a small maven build for debugging purpose (only the real core parts!), you may 

$> git-clone http://ns1.backwork.net/git/eclipselink.git
$> git-submodule init 
$> git-submodule update
$> cd eclipselink
$> mvn clean install

my examples are contained in the org.eclipse.persistence.jpa module under /IT
Please tell me if I should add additional license information, or if anything else is missing (if someone cares). I'll update the sources from svn regulary.

There are 2 eclipse OSGI jars bundled with eclipselink which are _not_ currently released officially? At least I did not find an official maven repo where I can find them. So these 2 jars have to be installed manually (maven tells you the command)
		<dependency>
			<groupId>org.osgi</groupId>
			<artifactId>osgi_R4_compendium</artifactId>
			<version>4.1.0.build-200702212030</version>
		</dependency>
		<dependency>
			<groupId>org.osgi</groupId>
			<artifactId>osgi_R4_core</artifactId>
			<version>4.1.0.build-200702212030</version>
		</dependency>


--- Guy Pelletier <guy.pelletier@xxxxxxxxxx> schrieb am Mo, 17.11.2008:

> Von: Guy Pelletier <guy.pelletier@xxxxxxxxxx>
> Betreff: Re: [eclipselink-dev] Question aboutEmbeddedAccessor#processEmbeddableClass()
> An: "Mark Struberg" <struberg@xxxxxxxx>, eclipselink-dev@xxxxxxxxxxx
> Datum: Montag, 17. November 2008, 20:25
> Hi Mark.
> 
> So I didn't try this but, here's what I'm
> thinking off the cuff (note: I didn't compile or
> validate this code)
> 
> From the 'owning' entity accessor, when processing
> the accessors check for embedded accessors and call a
> processPrefix method it:
> 
> if (accessor.isEmbedded()) {
>     accessor.processPrefix(accessor.getPrefix(),
> getProject().getEmbeddableAccessor(accessor.getReferenceClass()),
> accessor.getDescriptor().getMappingForAttributeName(accessor.getAttributeName));
> }
> 
> where the process prefix method looks something like:
> 
> protected void processPrefix(String prefix,
> EmbeddableAccessor accessor, AggregateObjectMapping mapping)
>  {
>   for (MappingAccessor accessor :
> accessor.getDescriptor().getAccessors()) {
>     if (accessor.isEmbedded()) {
>       ((EmbeddedAccessor) accessor).processPrefix(prefix +
> accessor.getPrefix(),
> getProject().getEmbeddableAccessor(accessor.getReferenceClass()),
> mapping);
>     } else {
>       if (prefix == null) {
>         // no prefix to apply ...
>         break;
>       } else { 
>         String attributeName = accessor.getAttributeName();
> 
>         DatabaseMapping aggregateMapping =
> getReferenceDescriptor().getMappingForAttributeName(attributeName);
>              
>         if (aggregateMapping.getField() != null) {
>           String sourceFieldName =
> aggregateMapping.getField().getName(); 
>           String prefixedFieldName = prefix +
> sourceFieldName;
>          
> mapping.addFieldNameTranslation(prefixedFieldName,
> sourceFieldName);
>         }
>       }
>     }
>   }
> }
> 
> Anyway, I'm thinking something along that line, likely
> still some issues to work out, but give it a try and see
> what you get.
> 
> Cheers,
> Guy
> 
> 
> ----- Original Message ----- 
> From: "Mark Struberg" <struberg@xxxxxxxx>
> To: <eclipselink-dev@xxxxxxxxxxx>; "Guy
> Pelletier" <guy.pelletier@xxxxxxxxxx>
> Sent: Monday, November 17, 2008 11:20 AM
> Subject: Re: [eclipselink-dev] Question
> aboutEmbeddedAccessor#processEmbeddableClass()
> 
> 
> Hi Guy!
> 
> Txs 4 the help. I tried to implement the functionality as
> you said, but while it works like a charm if I only have 1
> layer deep embeddings, I get problems with nested ones.
> 
> Maybe you can take a look at the attached patch (I wrote a
> few invoker IT's for this situation, which I will
> published later on after I did some cleanup)?
> 
> The problematic situation:
> 
> @Embeddable
> public class Price {
> @Column(precision=12, scale=2)
> private BigDecimal amount;
> private String     currency;
> }
> 
> @Embeddable
> public class TaxedPrice {
> @Embedded(prefix = "N_") private Price netPrice;
> @Embedded(prefix = "G_") private Price
> grossPrice;
> }
> 
> @Entity
> public class PriceWatch {
> @Id @GeneratedValue private int id;
> @Embedded(prefix = "H_") private TaxedPrice
> highestPrice;
> @Embedded(prefix = "L_") private TaxedPrice
> lowestPrice;
> }
> 
> 
> So for the table PriceWatch I'd like to get the
> following columns:
> ID
> H_N_AMOUNT
> H_N_CURRENCY
> H_G_AMOUNT
> H_G_CURRENCY
> L_N_AMOUNT
> L_N_CURRENCY
> L_G_AMOUNT
> L_G_CURRENCY
> 
> Since the innermost classes don't have the information
> of the prefixes L_ and H_, this info has to be
> 'pushed' into the objects recursively afterwards,
> which is utterly broken the way I tried it. I found no way
> to handle me down the embeddings to the fields e.g. via:
> 
>                 if (aggregateMapping instanceof
> AggregateObjectMapping) {
>                 AggregateObjectMapping aggregateObjMapping
> = (AggregateObjectMapping) aggregateMapping;
>                 Map<String, String> m =
> aggregateObjMapping.getAggregateToSourceFieldNames();
>                 
>                 // add the m_prefix to all the mappings 
>                 for(Map.Entry<String, String>
> fieldMapping : m.entrySet()) {
>                 fieldMapping.setValue(m_prefix +
> fieldMapping.getValue());
>                 }
>                 }
> 
> it seems that I always get the same attributeMapping for
> all embeddings which leads to:
> INSERT INTO PRICEWATCH (ID, L_H_N_AMOUNT, L_H_N_CURRENCY,
> L_H_G_AMOUNT, L_H_G_CURRENCY, L_H_N_AMOUNT, L_H_N_CURRENCY,
> L_H_G_AMOUNT, L_H_G_CURRENCY)...
> 
> An alternative handling would be to pass the prefix up the
> chain, so all prefixes would stack up almost automatic.
> But this would require the fix for each embedding has
> it's own EmbeddableAccessor first.
> 
> EmbeddedAccessor.java line # 393 something like:
> 
>            
> accessor.setOwningDescriptor(getOwningDescriptor());
> +           accessor.setPrefix(m_prefix);
>             accessor.process();
>             accessor.setIsProcessed();    
> 
> and m_prefix stacking up himself like
> +           m_prefix += (embedded == null) ? "" :
> (String) MetadataHelper.invokeMethod("prefix",
> embedded);
> 
> 
> But I think there must be somethings much more simple than
> that. It's getting to complicated. An old lore says:
> there is no problem which can't be solved by adding an
> additional step of indirection, but I'm not a friend of
> these 'bypass' operations.
> 
> Maybe you have some tip for me again?
> 
> txs and LieGrue,
> strub
> 
> 
> 
> --- Guy Pelletier <guy.pelletier@xxxxxxxxxx> schrieb
> am Mi, 12.11.2008:
> 
> > Von: Guy Pelletier <guy.pelletier@xxxxxxxxxx>
> > Betreff: Re: [eclipselink-dev] Question
> aboutEmbeddedAccessor#processEmbeddableClass()
> > An: "Mark Struberg"
> <struberg@xxxxxxxx>, eclipselink-dev@xxxxxxxxxxx
> > Datum: Mittwoch, 12. November 2008, 20:35
> > Hi Mark,
> > 
> > So I wouldn't use the word store, but rather a new
> > EmbeddableAccessor should be created and processed for
> each
> > usage of an Embedded. Internally (beyond the metadata
> > processing objects) EclipseLink uses/has only one
> class
> > descriptor to every class (entity or embeddable) of
> the
> > persistence unit.
> > 
> > I took a  look at your prefix annotation. I would
> think you
> > could achieve this functionality by adding another
> > processing method to EmbeddedAccessor to iterate over
> the
> > mappings of the embeddable class (once it has been
> > processed) and add a field name translation (just like
> we do
> > for attribute overrides) using the prefix to each
> field of
> > the mappings of the embeddable? If there is a nested
> case,
> > you would bury down into its mappings and apply the
> prefix,
> > unless the nested embedded defined its own prefix, and
> so
> > on.
> > 
> > So I'm not sure if in your case you need to worry
> about
> > the notion of the owning descriptor? The notion of the
> > owning descriptor was introduced post JPA 1.0 to
> facilitate
> > embeddables (nested or not) to specify metadata beyond
> what
> > was allowed in the spec. Namely things like Id's,
> > GeneratedValue's etc .. those items which required
> > metadata settings on to the owning entity.
> Unfortunately in
> > the current design those metadata items are only
> processed
> > for the first entity that uses the shared embedded
> (this is
> > a bug, which I pointed out earlier).
> > 
> > The model you outlined below should work no problem
> (it
> > doesn't have owning descriptor dependencies)
> > 
> > I hope this helps!
> > 
> > Cheers,
> > Guy
> > 
> > ----- Original Message ----- From: "Mark
> > Struberg" <struberg@xxxxxxxx>
> > To: <eclipselink-dev@xxxxxxxxxxx>; "Guy
> > Pelletier" <guy.pelletier@xxxxxxxxxx>
> > Sent: Wednesday, November 12, 2008 12:29 PM
> > Subject: Re: [eclipselink-dev] Question
> > aboutEmbeddedAccessor#processEmbeddableClass()
> > 
> > 
> > Hi Guy!
> > 
> > Txs 4 the quick response!
> > 
> > So (I better repeat this to make sure I understood it
> > correctly) the question is: should we store the
> aggregated
> > EmbeddableAccessor once for each table, or once for
> every
> > usage of @Embedded, correct?
> > 
> > As you've maybe read, I'm currently working on
> a
> > prefix annotation for embedded classes. If this will
> be part
> > of @Embedded or an own annotation doesn't imho
> change
> > much, in any case we have add the m_prefix handling to
> the
> > EmbeddedAccessor. So, I'm sure there is another
> way in
> 
> > the chain to add the prefix to the column names, but
> my gut
> > feeling tells me that this would be much easier to
> implement
> > if there is an EmbeddableAccessor for every Embedding.
> > Including nested embeddings.
> > 
> > Btw, what about nested embeddings currently?
> > I did not debug through, but can you imagine a
> situation
> > where we do also need the info parted in this case?
> > 
> > An example:
> > 
> > @Embeddable
> > public class Price {
> > private BigDecimal amount;
> > private String     currency;
> > }
> > 
> > @Embeddable
> > public class ListedPrice {
> > private int someValue;
> > 
> > @Embedded
> > @SomeFancyPriceModification
> > private Price lPrice;
> > }
> > 
> > @Entity
> > public class Purchase {
> > @Id @GeneratedValue private int id;
> > 
> > @Embedded
> > private ListedPrice listedPrice;
> > 
> > @Embedded
> > @AnotherFancyPriceModification
> > private Price soldPrice;
> > }
> > 
> > 1.) Are there any valid annotations which may affect
> the
> > fields? I know about the AttributeOverrides which has
> a
> > special handling in EmbeddedAccessor, so this should
> not be
> > a problem for @Embedding. But what happens if
> > AttributeOverrides is being used inside a nested class
> > (ListedPrice)?
> > 2.) If so, may this be handled if we only have one
> > EmbeddableAccessor per OwningDescriptor?
> > 
> > Humm, my brain is smoking, hope this post isn't
> getting
> > too confusing ;)
> > 
> > 
> > LieGrue,
> > strub
> > 
> > P.S.:
> > And a few additional thoughts coming to my mind:
> > a) What would be the downside of storing the
> > EmbeddableAccesor for each @Embedded usage? We have to
> store
> > one for each entity anyway. Technically his would imho
> only
> > have an effect if there are many embeddings of the
> same type
> > in the same table. Logically it is simply a difference
> if an
> > Accessor represents a 'database archetype' or
> a
> > specific usage of this archetype.
> > b) Could we split the EmbeddedAccessor in a
> > 'static' part and a part for each embedding?
> > c) What about adding back-references to the actual
> > EmbeddedAccessor for each processing step and stay
> with 1
> > EmbeddableAccessor (which then has to lookup varying
> parts
> > from it's embedding)?
> > 
> > 
> > --- Guy Pelletier <guy.pelletier@xxxxxxxxxx>
> schrieb
> > am Mi, 12.11.2008:
> > 
> > > Von: Guy Pelletier
> <guy.pelletier@xxxxxxxxxx>
> > > Betreff: Re: [eclipselink-dev] Question
> > aboutEmbeddedAccessor#processEmbeddableClass()
> > > An: eclipselink-dev@xxxxxxxxxxx
> > > Datum: Mittwoch, 12. November 2008, 16:05
> > > Hi Mark,
> > > 
> > > Happy to hear you're enjoying working with
> the
> > code.
> > > 
> > > What you have pointed out is indeed a problem and
> is
> > > problamatic for a number of use cases. See the
> > following bug
> > >
> (https://bugs.eclipse.org/bugs/show_bug.cgi?id=250144)
> > > 
> > > The notion of the owning descriptor should remain
> > however
> > > (e.g. an id field defined on an embeddable
> requires
> > setting
> > > metadata on the actual owning entity) , but the
> > problem as
> > > you have pointed out is that the embeddable class
> is
> > only
> > > processed once from the first entity that
> references
> > it. So
> > > this is incorrect. The embeddable class should
> likely
> > be
> > > cloned (similarly as we do with mapped
> superclasses)
> > and
> > > re-processed using the context or each owning
> > descriptor.
> > > Note, the whole class likely will not need to be
> > > re-processed, likely only the accessors and again
> > you'll
> > > have to be careful as to which accessors gets
> > re-processed
> > > since you don't want all the metadata from
> your
> > > embeddable class stored on the owning descriptor.
> > > 
> > > See the reloadMappedSuperclass method from
> ORMetadata.
> > > Notice, it clones a mapped superclass by
> reloading it
> > from
> > > XML. In your case you would have to handle the
> case
> > where
> > > the Embeddable was not loaded from XML (the
> accessible
> > > object will not have an associated entity
> mappings
> > object)
> > > and you'd likely just have to create a new
> > instance of
> > > the EmbeddableAccessor. I've just recently
> built a
> > > similar method for Entities (in my current work
> for
> > > TABLE_PER_CLASS support, see below)
> > > 
> > > /**
> > >  * INTERNAL:
> > >  * This method should be called to reload an
> entity
> > (that
> > > was either
> > >  * loaded from XML or an annotation) as a way of
> > cloning
> > > it. This is needed
> > >  * when we process TABLE_PER_CLASS inheritance.
> We
> > must
> > > process the parent
> > >  * classes for every subclasses descriptor. The
> > processing
> > > is similar to
> > >  * that of processing a mapped superclass, in
> that we
> > > process the parents
> > >  * with the subclasses context (that is, the
> > descriptor we
> > > are given).
> > > */
> > > protected EntityAccessor
> reloadEntity(EntityAccessor
> > > entity, MetadataDescriptor descriptor) {
> > >     if (m_accessibleObject.getEntityMappings() ==
> > null) {
> > >         // Create a new EntityAccesor.
> > >         EntityAccessor entityAccessor = new
> > > EntityAccessor(entity.getAnnotation(),
> > > entity.getJavaClass(), entity.getProject());
> > >         // Things we care about ...
> > >         entityAccessor.setDescriptor(descriptor);
> > > 
> > >
> >
> entityAccessor.getDescriptor().setDefaultAccess(entity.getDescriptor().getDefaultAccess());
> > >         return entityAccessor;
> > >     } else {
> > >         return
> > >
> >
> m_accessibleObject.getEntityMappings().reloadEntity(entity,
> > > descriptor);
> > >     }
> > > }
> > > 
> > > Hope this helps!
> > > 
> > > Cheers,
> > > Guy
> > > 
> > > 
> > > ----- Original Message ----- From: "Mark
> > Struberg" <struberg@xxxxxxxx>
> > > To: <eclipselink-dev@xxxxxxxxxxx>
> > > Sent: Wednesday, November 12, 2008 6:04 AM
> > > Subject: [eclipselink-dev] Question
> > > aboutEmbeddedAccessor#processEmbeddableClass()
> > > 
> > > 
> > > Hi!
> > > 
> > > I've read through the eclipselink sources and
> like
> > to
> > > add a few features.
> > > 
> > > Generally, I'd to say that it is a really
> well
> > done and
> > > _very_ readable source! I did look at the sources
> for
> > only 2
> > > evenings now (and I really enjoyed digging into
> it),
> > so
> > > don't be rude if I completely
> misinterpreted/not
> > > understood something yet :)
> > > 
> > > There's a point in the EmbeddedAccesor which
> I do
> > not
> > > understand:
> > > 
> > > In the function processEmbeddableClass(), a
> > > 'cached' EmbeddedAccessor instance is
> > retrieved
> > > (line # 299)
> > >         EmbeddableAccessor accessor =
> > >
> >
> getProject().getEmbeddableAccessor(getReferenceClassName());
> > > 
> > > So I'll get the same EmbeddableAccessor
> instance
> > for
> > > EVERY embedding of the same class.
> > > 
> > > A bit later in the source (line # 349), this
> > > EmbeddableAccessor will be filled with the
> > OwningDescriptor:
> > > 
> > >
> accessor.setOwningDescriptor(getOwningDescriptor());
> > > 
> > > BUT: imho the owning descriptor may be different
> for
> > each
> > > @Embedded !?
> > > 
> > > In praxis this means, that the EmbeddableAccessor
> > always
> > > points to the Table-Descriptor of the
> 'first'
> > > occurence (which one this ever may be ->
> random
> > > generator?), and the other ones may simply
> contain
> > wrong
> > > values.
> > > 
> > > My example:
> > > 
> > > @Embeddable
> > > public class Price {
> > > private BigDecimal amount;
> > > private String     currency;
> > > }
> > > 
> > > @Entity
> > > public class Purchase {
> > > @Id
> > > @GeneratedValue
> > > private int id;
> > > 
> > > @Embedded(prefix = "N_")
> > > private Price netPrice;
> > > 
> > > @Embedded(prefix = "G_")
> > > private Price grossPrice;
> > > }
> > > 
> > > @Entity
> > > public class OtherUseOfPrice {
> > > @Id
> > > @GeneratedValue
> > > private int id;
> > > 
> > > @Embedded(prefix = "O_")
> > > private Price otherPrice;
> > > }
> > > 
> > > In the debugger I got the following memory
> addresses:
> > > 
> > > O_ id=40 accessor=98 getowningDescriptor()= 45
> > > N_ id=101 accessor=98 getowningDescriptor()= 102
> > > G_ id=116 accessor=98 getowningDescriptor()= 102
> > > 
> > > And as expected, the accessor of the embeddings
> of
> > netPrice
> > > (N_) and grossPrice (G_) point to the wrong
> > > m_owningDescriptor 45 instead of the correct 102
> > > 
> > > Does this have any impact on the result? If not,
> we
> > also
> > > could remove the owningDescriptor from the
> > > EmbeddableAccessor, otherwise it may be a source
> of
> > > problems, isn't?






Back to the top