Uploaded image for project: 'XWiki Rendering'
  1. XWiki Rendering
  2. XRENDERING-809

MacroDescriptor legacy getDefaultCategory() aspect is never weaved

    XMLWordPrintable

Details

    • Unit
    • Unknown
    • N/A
    • N/A

    Description

      Problem

      The backward-compatibility aspect MacroDescriptorAspect (in the xwiki-rendering-legacy-transformation-macro module) defines an @Around advice aroundGetDefaultCategory whose purpose is to make the deprecated MacroDescriptor.getDefaultCategory() return a value derived from the modern getDefaultCategories().

      This advice is never weaved. The build emits (only as a warning):

      advice defined in org.xwiki.rendering.macro.descriptor.MacroDescriptorAspect has not been applied [Xlint:adviceDidNotMatch]
      

      The pointcut is execution(String MacroDescriptor.getDefaultCategory()) && within(MacroDescriptor), but getDefaultCategory() has no method body inside MacroDescriptor: it exists only as a default method on CompatibilityMacroDescriptor (added to MacroDescriptor through @DeclareParents) and as an inter-type declaration on AbstractMacroDescriptor. So the join point targeted by within(MacroDescriptor) does not exist and the advice matches nothing.

      User-visible impact

      For any MacroDescriptor implementation that implements only the modern getDefaultCategories() and does not extend AbstractMacroDescriptor, the deprecated getDefaultCategory() returns null instead of one of the configured categories. Legacy code still calling getDefaultCategory() therefore silently loses the macro's default category, breaking the backward-compatibility guarantee.

      Fix

      Retarget the pointcut to CompatibilityMacroDescriptor (where the method body actually lives) and add a !cflowbelow(...) guard so that this advice and the symmetric getDefaultCategories() advice do not recurse infinitely when a descriptor implements neither method. Add unit tests covering both the derivation and the neither-implemented cases.

      Note: this went unnoticed because AspectJ only reports an unmatched advice as a warning. See the companion Commons issue about failing the build in that case.

      Attachments

        Issue Links

          Activity

            People

              vmassol Vincent Massol
              vmassol Vincent Massol
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: