Uploaded image for project: 'Alfresco'
  1. Alfresco
  2. ALF-21806

removeAspect deletes properties inherited from base aspect even if base aspect is still applied

    Details

    • Type: Bug
    • Status: Won't Fix (View Workflow)
    • Priority: Unprioritized
    • Resolution: Unresolved
    • Affects Version/s: Community Edition 201605 GA
    • Fix Version/s: None
    • Security Level: external (External user)
    • Labels:
      None
    • Security Severity:
      None

      Description

      Given the following constellation:

      • base aspect A defines property X
      • aspect B defined to have aspect A as parent
      • aspect A and B are applied to a node
      • property X is set with a value
      • aspect B is removed from the node (aspect A remains applied)

      The NodeService operation removeAspect will remove all the properties of aspect B including those inherited from the aspect A, even though aspect A is still applied. This may cause integrity errors as such property may be mandatory and enforced.

      Steps to reproduce
      1. Create custom model M1 via model manager
      2. Create aspect A in model M1
      3. Define property X (d:text) in aspect A
      4. Define form for aspect A
      5. Activate model M1
      6. Create custom model M2 via model manager
      7. Create aspect B in model M2 with parent aspect A
      8. Activate model M2
      9. Apply aspects A and B to a document in the Repository
      10. Set value of property X via Edit Metadata
      11. Remove aspect B
      12. Check metadata

      Expected result: property X (defined by aspect A) is still present with the previous value
      Observed result: property X is displayed as having no value

      Note: The steps to reproduce involve two separate models due to a limitation in the model manager (ALF-21807).

        Attachments

          Activity

          Hide
          resplin Richard Esplin added a comment -

          Derek Hulley this looks like a subtle issue. Do you think it needs to be fixed in a service pack?

          Show
          resplin Richard Esplin added a comment - Derek Hulley this looks like a subtle issue. Do you think it needs to be fixed in a service pack?
          Hide
          dhulley Derek Hulley added a comment - - edited

          It is not so much subtle as unfortunate. This is, in my view, not a bug: the system has always done this. Aspect derivation is fine but applying a base and derived aspect to the same node is weird. In the case where someone wants to do it, the result is that we remove the properties without looking to see if there is another aspect that needs them.

          A feature enhancement; could break existing applications that do the same unfortunate thing.

          Show
          dhulley Derek Hulley added a comment - - edited It is not so much subtle as unfortunate. This is, in my view, not a bug: the system has always done this. Aspect derivation is fine but applying a base and derived aspect to the same node is weird. In the case where someone wants to do it, the result is that we remove the properties without looking to see if there is another aspect that needs them. A feature enhancement; could break existing applications that do the same unfortunate thing.
          Hide
          resplin Richard Esplin added a comment -

          I appreciate the feedback Axel Faust, as I wasn't previously aware of this behavior.

          Given that this behavior has always existed, and that changing it would require fundamental changes to the product, we won't look to change it unless we get the request from a broad set of users. Now that I am aware, I'll continue to monitor feedback on this part of the product so that we can reevaluate if this is something we should do in the future.

          For now I will close this as "Won't Fix".

          Show
          resplin Richard Esplin added a comment - I appreciate the feedback Axel Faust , as I wasn't previously aware of this behavior. Given that this behavior has always existed, and that changing it would require fundamental changes to the product, we won't look to change it unless we get the request from a broad set of users. Now that I am aware, I'll continue to monitor feedback on this part of the product so that we can reevaluate if this is something we should do in the future. For now I will close this as "Won't Fix".
          Hide
          resplin Richard Esplin added a comment -

          Closing as Won't Fix, as this won't be a priority to address in the near future.

          Show
          resplin Richard Esplin added a comment - Closing as Won't Fix, as this won't be a priority to address in the near future.
          Hide
          afaust Axel Faust added a comment -

          Can there be at least a documentation task derived from this? If this behiaviour is to stay as it is, there should ideally be a change to data model validation that either forbids this unsupported constellation or warns about it when a model is loaded (like other log file warnings the system emits).

          Show
          afaust Axel Faust added a comment - Can there be at least a documentation task derived from this? If this behiaviour is to stay as it is, there should ideally be a change to data model validation that either forbids this unsupported constellation or warns about it when a model is loaded (like other log file warnings the system emits).
          Hide
          resplin Richard Esplin added a comment -

          It is hard to figure out the right place to document these sorts of edge cases such that they don't overwhelm new developers while being available at the right place and time.

          Would you be willing to write this up as a blog post in community.alfresco.com?

          Show
          resplin Richard Esplin added a comment - It is hard to figure out the right place to document these sorts of edge cases such that they don't overwhelm new developers while being available at the right place and time. Would you be willing to write this up as a blog post in community.alfresco.com?
          Hide
          afaust Axel Faust added a comment -

          "It is hard to figure out the right place to document [this] [...]" - It's actually not because that is what JavaDoc is for: documenting pre-/post-conditions and side-effects of a Java-based API. The method JavaDoc already states "Remove an aspect and all related properties from a node." This can be easily expanded by adding "All properties defined by the aspect or inherited from any parents are considered to be 'related' properties. This operation will not ensure node integrity is maintained if any parent aspect defining properties is also applied on the node."

          Show
          afaust Axel Faust added a comment - "It is hard to figure out the right place to document [this] [...] " - It's actually not because that is what JavaDoc is for: documenting pre-/post-conditions and side-effects of a Java-based API. The method JavaDoc already states "Remove an aspect and all related properties from a node." This can be easily expanded by adding "All properties defined by the aspect or inherited from any parents are considered to be 'related' properties. This operation will not ensure node integrity is maintained if any parent aspect defining properties is also applied on the node."

            People

            • Assignee:
              Unassigned
              Reporter:
              afaust Axel Faust
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Date of First Response: