Re: code review for issue 743
Looks good to me.
-Tom
Michael Bouschen wrote:
>Hi Jielin,
>
>looks good! Two minor remarks:
>- The javadoc in TypeHelper.isEmbeddable talks about "embedded entity
>class". I propose to skip the word entity.
>- I propose to rename method EJBQLException.notSupportJoinArgument to
>EJBQLException.unsupportedJoinArgument (and the same name change for the
>corresponding constant).
>
>Regards Michael
>
>
>
>>Hi, Michael/Tom,
>>
>>Here is the change based on previous comments. I added
>>isEmbeddedAttribute(Object ownerClass, String attribute) in TypeHelper
>>and use this API in JoinDeclNode and FetchJoinNode for validation.
>>
>>Please review the change.
>>
>>Thanks.
>>
>>Jielin
>>
>>Tom Ware wrote:
>>
>>
>>
>>>Hi,
>>>
>>> Sahoo is right. Embeddable is called Aggregate in TopLink.
>>>isAggregateMapping() should provide this information.
>>>
>>>-Tom
>>>
>>>Sanjeeb Kumar Sahoo wrote:
>>>
>>>
>>>
>>>>Hi Michael,
>>>>
>>>>Let me try answering your question. Someone like Tom can override...
>>>>
>>>>public boolean isEmbeddedAttribute(Object ownerClass, String
>>>>attribute) {
>>>> ClassDescriptor desc = session.getDescriptor((Class)type);
>>>> return
>>>>desc.getMappingForAttributeName(attribute).isAggregateMapping();
>>>>}
>>>>
>>>>I don't think you need isEmbeddable() anymore. A validation exception
>>>>gets reported when an embedded attribute's type is not an embeddable.
>>>>
>>>>Thanks,
>>>>Sahoo
>>>>Michael Bouschen wrote:
>>>>
>>>>
>>>>
>>>>
>>>>>Hi Jielin,
>>>>>
>>>>>I'm wondering whether we have to distinguish @Embeddable and
>>>>>@Embedded here. I think an entity class might have an @Embeddable
>>>>>annotation. So the TypeHelper method taking a type representation
>>>>>should be called isEmbeddable:
>>>>>public boolean isEmbeddable(Object type);
>>>>>
>>>>>I think we need another TypeHelper method that checks whether a
>>>>>field or property is annotated with the @Embedded annotation, so
>>>>>something like:
>>>>>public Object isEmbeddedAttribute(Object ownerClass, String attribute);
>>>>>The method is called in the validate method of JoinDeclNode and
>>>>>FetchJoinNode. The path property of the two nodes is a DotNode. The
>>>>>type of the left child is the ownerClas, the name of the right child
>>>>>(an AttributeNode) is the attribute parameter for the call.
>>>>>
>>>>>I'm not sure how the @Embedded annotation is represented in the
>>>>>TopLink mapping classes, so I do not know how to implement method
>>>>>isEmbeddedAttribute. Tom, do you know?
>>>>>
>>>>>Regards Michael
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>Hi, Michael/Tom,
>>>>>>
>>>>>>Issue 743 Join of embedded entities should be disallowed is fix in
>>>>>>this jar. Also the unit test for this issue is included. Would you
>>>>>>please review the code?
>>>>>>
>>>>>>The changes are:
>>>>>>
>>>>>>Added isEmbeddedEntity() API.
>>>>>>entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/TypeHelperImpl.java
>>>>>>
>>>>>>entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/TypeHelper.java
>>>>>>
>>>>>>
>>>>>>Added code for check if the entity is embedded or not
>>>>>>entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/JoinDecNode.java
>>>>>>
>>>>>>entity-persistence/src/java/oracle/toplink/essentials/internal/parsing/FetchJoinNode.java
>>>>>>
>>>>>>
>>>>>>Added EJBQL error message:
>>>>>>entity-persistence/src/java/oracle/toplink/essentials/exceptions/EJBQLException.java
>>>>>>
>>>>>>entity-persistence/src/java/oracle/toplink/essentials/exceptions/i18n/EJBQLExceptionResource.java
>>>>>>
>>>>>>
>>>>>>Added unit test
>>>>>>entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/ejb/ejbqltesting/JUnitEJBQLValidationTestSuite.java
>>>>>>
>>>>>>
>>>>>>Thanks.
>>>>>>
>>>>>>Jielin
>>>>>>
>>>>>>
>>>>>>