Trails
  1. Trails
  2. TRAILS-81

regression:OneToOne decoration faulty...HARD OneToOne associations broken

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.1.0
    • Component/s: trails-hibernate
    • Labels:
      None
    • Environment:
      All
    • Number of attachments :
      1

      Description

      AssociationSelect does not support soft associations anymore

      Previous revisions of trails-1.1-SNAPSHOT supported a version of HibernateDescriptorDecorator that accomodated HARD and SOFT OneToOne associations.

      Hard meaning the OneToOne property editor would render.
      Soft meaning the simple AssociationSelect property editor would render.

      Currently, recent modifications have regressed to just the former. Latter logic is no longer working.

      It is all based on the parsing of the mappedBy annotation field and the determination of whether to instantiate ObjectReferenceDescriptor or OwningObjectReferenceDescriptor.

      Former logic operated as follows:

      If mappedBy = "" then instantiate ObjectReferenceDescriptor.
      If mappedBy= "something" then drill into class reflection and find the owner and it's associated property and instantiate OwningObjectReferenceDescriptor using those attributes retrieved from class reflection.

      The code for this was pain staking and valuable.

      It would be nice to see it working asap.

        Activity

        Hide
        Kenneth William Colassi added a comment -

        Please refer to revision 554 for working logic.

        Show
        Kenneth William Colassi added a comment - Please refer to revision 554 for working logic.
        Hide
        Alejandro Scandroli added a comment -

        I see, I misinterpreted the logic.
        But now that I do understand it, it doesn't look right to implicitly override the hibernate behavior.
        I think you should introduce a new annotation to handle this. This way the user would need to explicitly tell Trails to use an OwningObjectReferenceDescriptor using this new annotation.
        Here you have information about creating a your own annotation: http://www.trailsframework.org/Adding+Custom+Editor

        Show
        Alejandro Scandroli added a comment - I see, I misinterpreted the logic. But now that I do understand it, it doesn't look right to implicitly override the hibernate behavior. I think you should introduce a new annotation to handle this. This way the user would need to explicitly tell Trails to use an OwningObjectReferenceDescriptor using this new annotation. Here you have information about creating a your own annotation: http://www.trailsframework.org/Adding+Custom+Editor
        Hide
        Kenneth William Colassi added a comment -

        Alejandro, can you post usage of this annotation as you see it? An example.

        I felt that the implicit overriding was ok and could always be extended thereafter.

        I'd like get a feel for what you had in mind for usage and make a decision fromt there.

        Thanks.

        Show
        Kenneth William Colassi added a comment - Alejandro, can you post usage of this annotation as you see it? An example. I felt that the implicit overriding was ok and could always be extended thereafter. I'd like get a feel for what you had in mind for usage and make a decision fromt there. Thanks.
        Hide
        Kenneth William Colassi added a comment -

        Owner-<>-----Association

        Ok, so if on owner we have

        @OneToOne(owner=true, association=false)

        Then this guy is the owner and HibernateDescriptorDecorator should allocate OwningObjectReferenceDescriptor right?

        @OneToOne(owner=false, association=true)

        Then this guy is the association and HibernateDescriptorDecorator should allocate ObjectReferenceDescriptor right?

        Not sure so...
        DEFAULT ORIGINAL LOGIC...
        By default everything should be allocated ObjectReferenceDescriptor.

        Then we check for mappedBy... if that exists then we drill into the class/reflection structures to get the actual class property within the owning object of the class being decorated. NOTE: The class currently being iterated by decorator could be owner or association.

        So I don't think tagging the owning class as owner will help because it is the association we are decorating.

        Is it ok if we stay with the original logic?

        Show
        Kenneth William Colassi added a comment - Owner- <> -----Association Ok, so if on owner we have @OneToOne(owner=true, association=false) Then this guy is the owner and HibernateDescriptorDecorator should allocate OwningObjectReferenceDescriptor right? @OneToOne(owner=false, association=true) Then this guy is the association and HibernateDescriptorDecorator should allocate ObjectReferenceDescriptor right? Not sure so... DEFAULT ORIGINAL LOGIC... By default everything should be allocated ObjectReferenceDescriptor. Then we check for mappedBy... if that exists then we drill into the class/reflection structures to get the actual class property within the owning object of the class being decorated. NOTE: The class currently being iterated by decorator could be owner or association. So I don't think tagging the owning class as owner will help because it is the association we are decorating. Is it ok if we stay with the original logic?
        Hide
        Kenneth William Colassi added a comment -

        Alejandro, The impact to the working code I checked in is too broad and spans too many classes. I feel like I am being sent back to first week of April to re-write it all. Support for the original logic is removed.

        Since the code is now your version, can you implement/patch?

        Show
        Kenneth William Colassi added a comment - Alejandro, The impact to the working code I checked in is too broad and spans too many classes. I feel like I am being sent back to first week of April to re-write it all. Support for the original logic is removed. Since the code is now your version, can you implement/patch?
        Hide
        Kenneth William Colassi added a comment -

        Ok I am trying to see your code thru to the solution.

        It appears this block is not working is mappedBy is not found. The soft OneToOne editor is not launched.

        Maybe your code is good and the editor bug is in the editor logic?

        IPropertyDescriptor descriptorReference =
        new ObjectReferenceDescriptor(type, descriptor, hibernateType.getReturnedClass());

        try
        {
        Field propertyField = parentClassType.getDeclaredField(descriptor.getName());
        PropertyDescriptor beanPropDescriptor = (PropertyDescriptor) Ognl
        .getValue("propertyDescriptors.{? name == '" + descriptor.getName() + "'}[0]",
        Introspector.getBeanInfo(parentClassType));
        Method readMethod = beanPropDescriptor.getReadMethod();
        String mappedBy = "";
        if (readMethod.isAnnotationPresent(javax.persistence.OneToOne.class))

        { mappedBy = readMethod.getAnnotation(javax.persistence.OneToOne.class).mappedBy(); }

        else if (propertyField.isAnnotationPresent(javax.persistence.OneToOne.class))

        { mappedBy = propertyField.getAnnotation(javax.persistence.OneToOne.class).mappedBy(); }

        else

        { return descriptorReference; }
        Show
        Kenneth William Colassi added a comment - Ok I am trying to see your code thru to the solution. It appears this block is not working is mappedBy is not found. The soft OneToOne editor is not launched. Maybe your code is good and the editor bug is in the editor logic? IPropertyDescriptor descriptorReference = new ObjectReferenceDescriptor(type, descriptor, hibernateType.getReturnedClass()); try { Field propertyField = parentClassType.getDeclaredField(descriptor.getName()); PropertyDescriptor beanPropDescriptor = (PropertyDescriptor) Ognl .getValue("propertyDescriptors.{? name == '" + descriptor.getName() + "'} [0] ", Introspector.getBeanInfo(parentClassType)); Method readMethod = beanPropDescriptor.getReadMethod(); String mappedBy = ""; if (readMethod.isAnnotationPresent(javax.persistence.OneToOne.class)) { mappedBy = readMethod.getAnnotation(javax.persistence.OneToOne.class).mappedBy(); } else if (propertyField.isAnnotationPresent(javax.persistence.OneToOne.class)) { mappedBy = propertyField.getAnnotation(javax.persistence.OneToOne.class).mappedBy(); } else { return descriptorReference; }
        Hide
        Kenneth William Colassi added a comment -

        Shouldn't descriptorReference be PropertyDescriptor when allocating OwningObjectReferenceDescriptor?

        IPropertyDescriptor descriptorReference =
        new ObjectReferenceDescriptor(type, descriptor, hibernateType.getReturnedClass());

        I see your allocating it by default at the top.

        Original logic I believe allocated PropertyDescriptor and then added the extension from there. I cannot comment on the significance of the difference.

        I am just trying to understand why the editor is not coming up like it should.

        Also, I removed all my mappedBy attributes from my pojos and the hard editor keeps coming up.

        Show
        Kenneth William Colassi added a comment - Shouldn't descriptorReference be PropertyDescriptor when allocating OwningObjectReferenceDescriptor? IPropertyDescriptor descriptorReference = new ObjectReferenceDescriptor(type, descriptor, hibernateType.getReturnedClass()); I see your allocating it by default at the top. Original logic I believe allocated PropertyDescriptor and then added the extension from there. I cannot comment on the significance of the difference. I am just trying to understand why the editor is not coming up like it should. Also, I removed all my mappedBy attributes from my pojos and the hard editor keeps coming up.
        Hide
        Kenneth William Colassi added a comment -

        Alejandro, your version does not support inverse.

        If you could patch this to work I would be greatful.

        thanks

        Show
        Kenneth William Colassi added a comment - Alejandro, your version does not support inverse. If you could patch this to work I would be greatful. thanks
        Hide
        Alejandro Scandroli added a comment -

        Can you post usage of this annotation as you see it? An example.

        Yes I can, but it may take a while since I don't have any free time these days.

        The impact to the working code I checked in is too broad and spans too many classes. I feel like I am being sent back to first week of April to re-write it all. Support for the original logic is removed.

        No, the wrong logic is only in HibernateDescriptorDecorator.decorateAssociationDescriptor every thing else is fine.

        Maybe your code is good and the editor bug is in the editor logic?

        No, HibernateDescriptorDecorator.decorateAssociationDescriptor is wrong. "mappedBy" is always "" when is not declared, so if there is a OneToOne annotation, the descriptor always end up decorated by an OwningObjectReferenceDescriptor.

        Shouldn't descriptorReference be PropertyDescriptor when allocating OwningObjectReferenceDescriptor?

        Nope, OwningObjectReferenceDescriptor is an extension now and it doesn't have all the information it needs to work. The information missing in the extension comes from the ObjectReferenceDescriptor. They complement each other.

        Alejandro, your version does not support inverse.

        Should it?

        Show
        Alejandro Scandroli added a comment - Can you post usage of this annotation as you see it? An example. Yes I can, but it may take a while since I don't have any free time these days. The impact to the working code I checked in is too broad and spans too many classes. I feel like I am being sent back to first week of April to re-write it all. Support for the original logic is removed. No, the wrong logic is only in HibernateDescriptorDecorator.decorateAssociationDescriptor every thing else is fine. Maybe your code is good and the editor bug is in the editor logic? No, HibernateDescriptorDecorator.decorateAssociationDescriptor is wrong. "mappedBy" is always "" when is not declared, so if there is a OneToOne annotation, the descriptor always end up decorated by an OwningObjectReferenceDescriptor. Shouldn't descriptorReference be PropertyDescriptor when allocating OwningObjectReferenceDescriptor? Nope, OwningObjectReferenceDescriptor is an extension now and it doesn't have all the information it needs to work. The information missing in the extension comes from the ObjectReferenceDescriptor. They complement each other. Alejandro, your version does not support inverse. Should it?
        Hide
        Alejandro Scandroli added a comment -

        Here you have a quick and dirty patch. TRAILS-81.v20070711.patch
        I think reflection is not needed now, but you tell me.
        If this works I will move this the decoration to a DescriptorAnnotationHandler whenever I have a chance.

        Show
        Alejandro Scandroli added a comment - Here you have a quick and dirty patch. TRAILS-81.v20070711.patch I think reflection is not needed now, but you tell me. If this works I will move this the decoration to a DescriptorAnnotationHandler whenever I have a chance.
        Hide
        Kenneth William Colassi added a comment -

        The issue of inverse is worrisome...

        My version supported it in order for the ognl to operate properly.

        Since yours does not support that... I am wondering if everything works as originally did with my version. I will run thru it for all cases when I come up with something workable.

        Sorry I went heavy on this jira... just wanted to get the attention it needed.

        I will check out your patch today and let you know results.

        Appreciate the help

        Show
        Kenneth William Colassi added a comment - The issue of inverse is worrisome... My version supported it in order for the ognl to operate properly. Since yours does not support that... I am wondering if everything works as originally did with my version. I will run thru it for all cases when I come up with something workable. Sorry I went heavy on this jira... just wanted to get the attention it needed. I will check out your patch today and let you know results. Appreciate the help
        Hide
        Alejandro Scandroli added a comment -

        What is the status of this issue?

        Show
        Alejandro Scandroli added a comment - What is the status of this issue?
        Hide
        Kenneth William Colassi added a comment -

        STATUS...

        I had dropped the ball on this since it's creation.

        The past 4 weeks have been hampered with ognl breakages and running the trails app has been close to impossible.

        Couple that with pre-emptions and there is not alot I have been able to reliably do with regard to operating this functionality (that being the OneToOne editor).

        I would like to resume and fix this unless someone has a better in to the code than I do.

        This bug basically requires that the OneToOne editor be re-looked-at and resolved since it was breaking after the tapestry-4.1.2 merge.

        Show
        Kenneth William Colassi added a comment - STATUS... I had dropped the ball on this since it's creation. The past 4 weeks have been hampered with ognl breakages and running the trails app has been close to impossible. Couple that with pre-emptions and there is not alot I have been able to reliably do with regard to operating this functionality (that being the OneToOne editor). I would like to resume and fix this unless someone has a better in to the code than I do. This bug basically requires that the OneToOne editor be re-looked-at and resolved since it was breaking after the tapestry-4.1.2 merge.
        Hide
        Kenneth William Colassi added a comment -

        Ok, the new behavior of this editor currently is the following asof current checkins:

        Click NEW on the OneToOne property (ie. and nothing happens.

        • no editor comes up
        • editor page blips/blinks
        • no behavior whatsoever
        Show
        Kenneth William Colassi added a comment - Ok, the new behavior of this editor currently is the following asof current checkins: Click NEW on the OneToOne property (ie. and nothing happens. no editor comes up editor page blips/blinks no behavior whatsoever
        Hide
        Kenneth William Colassi added a comment -

        Needed to clarify the title to this...

        Although soft associations by AssociationSelect may have been mentioned, impacted and subsequently resolved throughout the framework, the main initiative behind this JIRA was the impact to hard associations implemented by the OneToOne property editor. They got broken post T-4.1 merge.

        The semantics surrounding the descriptor decorator are under review.

        The main problem is that the case logic currently is resulting in the wrong desciptor being allocated at bootstrap.

        Show
        Kenneth William Colassi added a comment - Needed to clarify the title to this... Although soft associations by AssociationSelect may have been mentioned, impacted and subsequently resolved throughout the framework, the main initiative behind this JIRA was the impact to hard associations implemented by the OneToOne property editor. They got broken post T-4.1 merge. The semantics surrounding the descriptor decorator are under review. The main problem is that the case logic currently is resulting in the wrong desciptor being allocated at bootstrap.
        Hide
        Kenneth William Colassi added a comment -

        This could use some hands on testing...

        If you see anything just give me a holler.

        Show
        Kenneth William Colassi added a comment - This could use some hands on testing... If you see anything just give me a holler.
        Hide
        Kenneth William Colassi added a comment -

        BTW:

        Originally I designed this to operate the @OneToOne attribute mappedBy which gave us a handle to the inverse that the developer codes to the class.

        This design has changed and I decided to go with the extra annotation @HardOneToOne for detection/isolation of logic. I would have preferred to instrument @OneToOne with Identity but was discouraged from that.

        For your pojos...
        To setup the hard owner we use two annotations
        @OneToOne
        @HardOneToOne(identity = Identity.OWNER)
        To setup the hard association we use two annotations
        @OneToOne
        @HardOneToOne(identity = Identity.ASSOCIATION)

        Here is the use model...

        OWNER-<>-----ASSOCIATION

        We will use what roster demo uses to explain...

        Organization--<>-------Director

        This says only one director can be anchored to any Organization.

        Here is the usage...

        Organization.JAVA
        @OneToOne(cascade = CascadeType.REFRESH)
        @HardOneToOne(identity = Identity.OWNER)
        @JoinTable(name = "join_table_Organization_Director", joinColumns = @JoinColumn(name = "director_fk", insertable = true, updatable = true, nullable = true), inverseJoinColumns =

        { @JoinColumn(name = "organization_fk", insertable = true, updatable = true, nullable = true) }

        )
        public Director getDirector()

        { return director; }

        Director.JAVA
        @OneToOne
        @HardOneToOne(identity = Identity.ASSOCIATION)
        @JoinTable(name = "join_table_Organization_Director", joinColumns = @JoinColumn(name = "organization_fk", insertable = true, updatable = true, nullable = true), inverseJoinColumns =

        { @JoinColumn(name = "director_fk", insertable = true, updatable = true, nullable = true) }

        )
        public Organization getOrganization()

        { return organization; }
        Show
        Kenneth William Colassi added a comment - BTW: Originally I designed this to operate the @OneToOne attribute mappedBy which gave us a handle to the inverse that the developer codes to the class. This design has changed and I decided to go with the extra annotation @HardOneToOne for detection/isolation of logic. I would have preferred to instrument @OneToOne with Identity but was discouraged from that. For your pojos... To setup the hard owner we use two annotations @OneToOne @HardOneToOne(identity = Identity.OWNER) To setup the hard association we use two annotations @OneToOne @HardOneToOne(identity = Identity.ASSOCIATION) Here is the use model... OWNER- <> -----ASSOCIATION We will use what roster demo uses to explain... Organization-- <> -------Director This says only one director can be anchored to any Organization. Here is the usage... Organization.JAVA @OneToOne(cascade = CascadeType.REFRESH) @HardOneToOne(identity = Identity.OWNER) @JoinTable(name = "join_table_Organization_Director", joinColumns = @JoinColumn(name = "director_fk", insertable = true, updatable = true, nullable = true), inverseJoinColumns = { @JoinColumn(name = "organization_fk", insertable = true, updatable = true, nullable = true) } ) public Director getDirector() { return director; } Director.JAVA @OneToOne @HardOneToOne(identity = Identity.ASSOCIATION) @JoinTable(name = "join_table_Organization_Director", joinColumns = @JoinColumn(name = "organization_fk", insertable = true, updatable = true, nullable = true), inverseJoinColumns = { @JoinColumn(name = "director_fk", insertable = true, updatable = true, nullable = true) } ) public Organization getOrganization() { return organization; }

          People

          • Assignee:
            Kenneth William Colassi
            Reporter:
            Kenneth William Colassi
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: