Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-22821: Allow having generic analyzers for different types of macro arguments #3846

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

michitux
Copy link
Contributor

Jira URL

https://jira.xwiki.org/browse/XWIKI-22821

Changes

Description

  • Add a new MacroParameterRequiredRightsAnalyzer role.
  • Add implementations for wiki content and macro content source references.

Clarifications

  • This eliminates the need for XWIKI-22798: The code macro is missing a required rights analyzer #3810.
  • As this fixes the bug of the missing required rights analyzer for the code macro, I propose backporting this improvement to the LTS branches.
  • I put the parameter type as type parameter of the new role, but this type isn't used in the role anywhere so I added an explicit ignore for the warnings this causes. I don't know if we have a better solution for this.

Screenshots & Video

Example analysis of the following content:

{{code language="none" source="script:foo"}}{{/code}}

{{info title="{{groovy~}~}
println(~"Hello from Title~")
{{/groovy~}~}"}}
Type your information message here.
{{/info}}

image

Executed Tests

LANG=C.UTF-8 mvn clean install -Pdocker,legacy,integration-tests,snapshotModules,quality -pl :xwiki-platform-security-requiredrights-default,:xwiki-platform-security-requiredrights-macro

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • stable-16.10.x
    • stable-16.4.x

…cro arguments

* Add a new MacroParameterRequiredRightsAnalyzer role.
* Add implementations for wiki content and macro content source
  references.
@tmortagne
Copy link
Member

I propose backporting this improvement to the LTS branches

+1

I put the parameter type as type parameter of the new role, but this type isn't used in the role anywhere so I added an explicit ignore for the warnings this causes. I don't know if we have a better solution for this.

No better idea on my side.

Copy link
Member

@tmortagne tmortagne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good (except for the surprising field I mentioned).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants