Lukasz Lenart created WW-5678:
---------------------------------
Summary: Tidy up the SecurityMemberAccess package-matching helper
names and visibility
Key: WW-5678
URL: https://issues.apache.org/jira/browse/WW-5678
Project: Struts 2
Issue Type: Improvement
Reporter: Lukasz Lenart
Fix For: 8.0.0
Follow-up to [WW-5674|https://issues.apache.org/jira/browse/WW-5674], deferred
to 8.0.0 because part of it is a source-breaking change.
h2. 1. The package-private overload shares a name with a public method
WW-5674 left {{SecurityMemberAccess}} with three related static methods:
{code:java}
public static boolean isClassBelongsToPackages(Class clazz, Set
matchingPackages) // public API
static boolean isClassBelongsToPackages(Class clazz, Set first, Set
second) // package-private
static boolean isPackageBelongsToPackages(String packageName, Set first,
Set second)
{code}
(Generics omitted above — the issue tracker mangles them.)
The two-argument form is public; the three-argument overload deliberately is
not, because it has a single caller ({{isClassAllowlisted}}) and its only test
lives in the same package. Publishing it would freeze it as {{struts2-core}}
API.
Sharing a name across the visibility boundary is a hazard: a contributor
tidying up "an inconsistent modifier" can widen the overload to {{public}} by
reflex and never realise they have added permanent API surface. Giving the
package-private overload a distinct name removes the trap. This part is not
source-breaking and could land earlier if convenient.
h2. 2. The names are grammatically awkward
{{isClassBelongsToPackages}} and {{isPackageBelongsToPackages}} read as "is
class belongs to packages". Something like {{classBelongsToAnyPackage}} /
{{packageMatchesAny}} would be clearer. The public one cannot be renamed before
a major release.
h2. 3. Two public statics have no callers outside the class
{{isClassBelongsToPackages(Class, Set)}} and {{toPackageName(Class)}} are both
{{public static}} on a public class, but a repository-wide search finds no
caller outside {{SecurityMemberAccess}} itself — not in core, not in any
plugin, not in the tests. They appear to be public by accident rather than by
design.
Narrowing them to package-private would shrink the surface that has to be
preserved. This _is_ source-breaking for anyone who happens to call them, hence
8.0.0.
h2. Why this matters now
WW-4759 is drawing the {{struts2-api}} boundary for 8.0.0. Deciding what in
{{SecurityMemberAccess}} is genuinely API is part of the same exercise, and it
is cheaper to settle it while that work is open than to inherit these three
methods into the new artefact unexamined.
h2. Constraints
{{SecurityMemberAccessPackageMatchingTest}} calls all three methods directly
and will need updating. Any rename must not disturb the frozen
{{legacyPrefixMatch}} and {{legacyToPackageName}} reference oracles in that
test — they are verbatim copies of the pre-WW-5674 implementation kept as
differential oracles, and they must continue to _not_ delegate to production
code.
Renaming or narrowing anything public needs a migration-guide entry.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
