Sahoo,
There must be an error with the CMP3Policy code that extracts the PK values from the PK object. (createPKVectorFromKey()). The type in the TopLink Query should be the Integer, not the PK class.
--Gordon
-----Original Message-----
From: Sanjeeb.Sahoo_at_Sun.COM [mailto:Sanjeeb.Sahoo_at_Sun.COM]On Behalf Of
Sanjeeb Kumar Sahoo
Sent: Friday, August 25, 2006 12:18 PM
To: persistence_at_glassfish.dev.java.net; Yorke Gordon J
Cc: Tom Ware
Subject: Re: part of an embedded object is read-only...
Hi Gordon,
Thanks for the explanation and suggestion. If I make the exact change as
you suggested, I was getting NPE because
((AggregateObjectMapping)mapping).getReferenceDescriptor().getObjectBuilder().getMappingForField(field)
can return null for read-only fields of the embeddable class. So, my
change is slightly different from your suggestion. Now, the inner loop
looks like this (there are a couple of questions for you in the code below):
// Add field to mapping association
for (Enumeration fields = mapping.getFields().elements();
fields.hasMoreElements();) {
DatabaseField field =
DatabaseField.class.cast(fields.nextElement());
if (!mapping.isReadOnly()) {
if(mapping.isAggregateObjectMapping()){
// for Embeddable class, we need to test
read-only status
// of individual fields in the embeddable
final ObjectBuilder aggregateObjectBuilder =
AggregateObjectMapping.class.cast(mapping).
getReferenceDescriptor().getObjectBuilder();
// look in the non-read-only fields mapping
final DatabaseMapping aggregatedFieldMapping =
aggregateObjectBuilder.getMappingForField(field);
if(aggregatedFieldMapping == null) {
// null indicates read-only field, because we
// looked for the mapping in non-readonly
collection
assert(aggregateObjectBuilder.getReadOnlyMappingsByField().get(field) !=
null);
Vector mappingVector =
(Vector)getReadOnlyMappingsByField().get(field);
if (mappingVector == null) {
mappingVector =
NonSynchronizedVector.newInstance();
getReadOnlyMappingsByField().put(field,
mappingVector);
}
// Should we add aggregateFieldMapping? This
is a question for Gordon.
mappingVector.add(mapping);
} else {
// Should we add aggregateFieldMapping? This
is a question for Gordon.
getMappingsByField().put(field, mapping);
}
} else { // not an embeddable mapping
if (getMappingsByField().containsKey(field)) {
session.getIntegrityChecker().handleError(DescriptorException.multipleWriteMappingsForField(field.toString(),
mapping));
} else {
getMappingsByField().put(field, mapping);
}
}
} else { // mapping is read-only
Vector mappingVector =
(Vector)getReadOnlyMappingsByField().get(field);
if (mappingVector == null) {
mappingVector = NonSynchronizedVector.newInstance();
getReadOnlyMappingsByField().put(field,
mappingVector);
}
mappingVector.add(mapping);
}
}
With this change, I am getting the following conversion exception,
because now we have mapped the OneToOneMapping as the primary key mapping:
Exception [TOPLINK-3001] (Oracle TopLink Essentials - 2006.8 (Build
060822)): oracle.toplink.essentials.exceptions.ConversionException
Exception Description: The object [Table1PK id(1)], of class [class
Table1PK], could not be converted to [class java.lang.Integer].
at
oracle.toplink.essentials.exceptions.ConversionException.couldNotBeConverted(ConversionException.java:51)
at
oracle.toplink.essentials.internal.helper.ConversionManager.convertObjectToInteger(ConversionManager.java:507)
at
oracle.toplink.essentials.internal.helper.ConversionManager.convertObject(ConversionManager.java:128)
at
oracle.toplink.essentials.internal.descriptors.ObjectBuilder.buildRowFromPrimaryKeyValues(ObjectBuilder.java:946)
at
oracle.toplink.essentials.queryframework.ReadObjectQuery.prepareForExecution(ReadObjectQuery.java:484)
at
oracle.toplink.essentials.queryframework.DatabaseQuery.execute(DatabaseQuery.java:603)
at
oracle.toplink.essentials.queryframework.ObjectLevelReadQuery.execute(ObjectLevelReadQuery.java:677)
at
oracle.toplink.essentials.queryframework.ObjectLevelReadQuery.executeInUnitOfWork(ObjectLevelReadQuery.java:731)
at
oracle.toplink.essentials.internal.sessions.UnitOfWorkImpl.internalExecuteQuery(UnitOfWorkImpl.java:2218)
at
oracle.toplink.essentials.internal.sessions.AbstractSession.executeQuery(AbstractSession.java:937)
at
oracle.toplink.essentials.internal.sessions.AbstractSession.executeQuery(AbstractSession.java:894)
at
oracle.toplink.essentials.internal.ejb.cmp3.base.EntityManagerImpl.findInternal(EntityManagerImpl.java:298)
at
oracle.toplink.essentials.internal.ejb.cmp3.base.EntityManagerImpl.findInternal(EntityManagerImpl.java:274)
at
oracle.toplink.essentials.internal.ejb.cmp3.EntityManagerImpl.find(EntityManagerImpl.java:130)
This is the same exception that *Wonsoek* was earlier reporting with his
patch as well.
How do we fix this? Can we add the read-only mapping as the primary key
mapping?
Thanks,
Sahoo
Gordon Yorke wrote:
> Sahoo,
> In responce to your questions:
> An ClassDesciptor in TopLink can be flagged as an Embeddable
> (TopLink called embeddables 'Aggregates'). This effectively makes the
> Descriptor an AggregateObjectDescriptor and results in special
> behaviour. Part of that behaviour is that the parent descriptor sees
> the AggregateObjectMapping each time it looks for a field mapped in
> the embeddable.
> We are not making the AggregateObjectMapping read-only in the
> metadata processor because the entire AggregateObjectMapping is not
> read-only. read-only in the AggregateObjectMapping is a convience method.
>
>
> No onto the issue. It is because the OneToOne is mapped within
> the Entity and not the Embeddable that the exception is occurring.
> Changing the code to check the aggregates mappings before placing the
> mapping within the list for that field should resolve the problem.
> something like:
>
> for (Enumeration fields = mapping.getFields().elements();
> fields.hasMoreElements();) {
> Object field = fields.nextElement();
> if (!mapping.isReadOnly()) {
> if (mapping.isAggregateObjectMapping()){
> DatabaseMapping aggMapping =
> ((AggregateObjectMapping)mapping).getReferenceDescriptor().getObjectBuilder().getMappingForField(field);
> if (aggMapping.isReadOnly){
> Vector mappingVector =
> (Vector)getReadOnlyMappingsByField().get(field);
> if (mappingVector == null) {
> mappingVector =
> oracle.toplink.essentials.internal.helper.NonSynchronizedVector.newInstance();
>
> getReadOnlyMappingsByField().put(field, mappingVector);
> }
> mappingVector.add(mapping);
> }else{
> getMappingsByField().put(field, mapping);
> }
> } else{
> if (getMappingsByField().containsKey(field)) {
>
> session.getIntegrityChecker().handleError(DescriptorException.multipleWriteMappingsForField(field.toString(),
> mapping));
> }else
> getMappingsByField().put(field, mapping);
> }
> }
> } else {
> Vector mappingVector =
> (Vector)getReadOnlyMappingsByField().get(field);
> if (mappingVector == null) {
> mappingVector =
> oracle.toplink.essentials.internal.helper.NonSynchronizedVector.newInstance();
> getReadOnlyMappingsByField().put(field,
> mappingVector);
> }
> mappingVector.add(mapping);
> }
> }
> May need some tweeking to compile and could probably be re-organised.
> --Gordon
>
> -----Original Message-----
> *From:* Sanjeeb.Sahoo_at_Sun.COM [mailto:Sanjeeb.Sahoo_at_Sun.COM]*On
> Behalf Of *Sanjeeb Kumar Sahoo
> *Sent:* Wednesday, August 09, 2006 1:36 AM
> *To:* persistence_at_glassfish.dev.java.net; Yorke Gordon J
> *Cc:* Tom Ware
> *Subject:* Re: part of an embedded object is read-only...
>
> Hi Gordon,
>
> Thanks for getting back. See response in line...
>
> Gordon Yorke wrote:
>> Hello Sahoo,
>> If you are getting the multiple writable mappings in this case then it seems the updatable=false, insertable = false is being ignored by the annotation processor, most likely because it is an embedded pk.
> As I mentioned earlier, I don't see any issue with the annotation
> processor code. It is correctly processing those flags.
>> Have you attempted to step through this code?
> Yes, I have.
>> Which mapping is causing the exception?
> I am posting the simplified version of code here so that I can
> explain my observation:
>
> void ObjectBuilder.initialize(AbstractSession session) {
> for (Mapping mapping : getDescriptor().getMappings() { // loop #1
> for(DatabaseField field : mapping.getFields()) { // loop
> #1.1
> if (!mapping.isReadOnly()) {
> if (getMappingsByField().containsKey(field)) {
> throw
> MultipleWritableMappingExistsException();
> } else {
> getMappingsByField().put(field, mapping);
> }
> } else {
> // add it to non-writable mappings vector
> ...
> }
> }
> }
> }
>
> Sequence(Table1.class):
> 1) loop #1 is entered, mapping =
> *AggregateObjectMapping*(isReadOnly = false).
> 2) loop #1.1 is entered, field = *DatabaseField* (column = "ID"
> and Table = "Table1", isUpdatable = isInsertable = false)
> 3) Since mappingsByField map is *empty* to start with, this field
> and corresponding AggregateMappingObject is put to the
> mappingsByField map. Point worth noting is that, for this field,
> *isUpdatable = isInsertable = false*. This is why I say,
> annotation processor has done its job. So, basically, this is
> where I see the *bug* is. We are adding a read-only field to a
> collection of non-read-only fields.
> 4) exit loop #1.1
> 5) enter loop #1 again, mapping = *OneToOneMapping*(isReadOnly =
> false)
> 6) loop 1.1 is entered, field = *DatabaseField* (column = "ID" and
> Table = "Table1", isUpdatable = isInsertable = true)
> 7) Since, mappingsByField already contains a (incorrect) mapping
> for this database field, we throw multipleWritableMappingException.
>> Has the mapping within the AggregateObjectDescriptor (the embedded's descriptor) been marked as read-only ?
>>
> I am not getting what you are asking. What is an
> AggregateObjectDescriptor? Did you mean AggregateObjectMapping? If
> you are asking about the read-only status of DatabaseField within
> the AggregateObjectMapping, then answer is yes. For the
> AggregateObjectMapping, the field is marked read-only.
>> Mappings within an Embeddable object can, individually, be marked as read-only. The entire object can also be marked read-only by making the embedded mapping (AggregateObjectMapping) read-only.
> As far as I know the code, we always set an AggregateObjectMapping
> as non-readonly. It is done unconditionally in
> EmbeddedAccessor.process(). So, I think, there is a difference
> between design and implementation. Do you agree? Let's track this
> discussion separately.
>> The code you have included is checking for multiple writable mappings. This would be multiple mappings mapped to the same database column both of which could cause the column to be updated on the database. TopLink saves users from confusion by requiring that only one of the mapped attributes be writable. The check on isAggregateObjectMapping() prevents the error from occuring if one of the subsequent mappings is within an Embedded object. This code could be expanded to detect multiple writable mappings within an Embeddable as well, but would probably not resolve our issue here.
>>
> You are right, that change won't fix the current issue. Now that
> you confirmed TopLink Essential can handle partially read-only
> embedded object, I can investigate further for a fix. As Wonsoek
> suggested, I tried changing:
> /if (!mapping.isReadOnly()) /
> to
> /if (!mapping.isReadOnly() && !field.isReadOnly()) /
>
> This results in following exception, because we have added a
> OneToOneMapping as the primaryKeyMapping, yet the primary key type
> is *int*:
> LINK-3001] (Oracle TopLink Essentials - 2006.7 (Build 060731)):
> oracle.toplink.essentials.exceptions.ConversionException
> Exception Description: The object [Table1PK id(92)], of class
> [class Table1PK], could not be converted to [class java.lang.Integer].
>
> I think, some change is required in
> ObjectBuilder.initializePrimaryKeys(). I will keep you posted, if
> I find any.
>
> Thanks,
> Sahoo
>> --Gordon
>>
>> -----Original Message-----
>> From: Sanjeeb.Sahoo_at_Sun.COM [mailto:Sanjeeb.Sahoo_at_Sun.COM]On Behalf Of
>> Sanjeeb Kumar Sahoo
>> Sent: Monday, August 07, 2006 10:27 AM
>> To: Tom Ware; Yorke Gordon J
>> Cc: persistence_at_glassfish.dev.java.net
>> Subject: part of an embedded object is read-only...
>>
>>
>> (Resending as the earlier email which had a wrong subject.)
>>
>> Hi Tom, Gordon,
>>
>> I am looking into issue
>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=894 . Looking at
>> the code I see that the metadata processing code is correctly processing
>> the insertable and updatable values that are specified in the Embeddable
>> class. An *AggregateObjectMapping* type of mapping is also created for
>> the embedded field. The bug seem to be in ObjectBuilder.java which is
>> reporting an incorrect exception. See code below from ObjectBuilder.java:
>> public void initialize(AbstractSession session) throws
>> DescriptorException {
>> ...
>> if (!mapping.isReadOnly()) {
>> if (getMappingsByField().containsKey(field)) {
>> // We cannot determine if aggregate mapping for
>> the field are read-only, so ignore exception.
>> if (!mapping.isAggregateObjectMapping()) {
>>
>> session.getIntegrityChecker().handleError(DescriptorException.multipleWriteMappingsForField(field.toString(),
>> mapping));
>> }
>> }
>> ...
>>
>> As you can see, from the above code, it is checking if the
>> AggregateObjectMapping is readonly or not. Should it not instead check
>> individual mapping objects that are part of the AggregateObjectMapping?
>> What I don't understand is why is their a boolean type field called
>> *isReadOnly* on an AggregateObjectMapping. What is the semantics of that
>> boolean flag? /Does TopLink Essential not support a part of an embedded
>> object to be read-only?/
>>
>> Thanks,
>> Sahoo
>>
>