[WFCORE-7258] Make use of ModuleIdentifierUtil.MODULE_NAME_CORRECTOR on the attributes that could require it

[mskacelik]

issue: https://issues.redhat.com/browse/WFCORE-7258

Hopefully I applied correctly all the correctors.

[yersan]

@mskacelik Added some comments, notice that the ultimate goal here by moving the module name corrector to the attributes, is also to avoid parsing to the canonize form twice ... it is a bit tricky, so we need to closely review this

[yersan]

Since the module will be provided now in a canonicalized form, apparently, we can now remove the canonicalization request done at

wildfly-core/elytron/src/main/java/org/wildfly/extension/elytron/ClassLoadingAttributeDefinitions.java

Line 46 in 481cbde

current = current.getModule(ModuleIdentifierUtil.parseCanonicalModuleIdentifier(module));

The only one I have in doubt is the invocation of that method from

wildfly-core/elytron/src/main/java/org/wildfly/extension/elytron/JaasCustomSecurityRealmWrapper.java

Line 63 in bb0806b

String moduleNameParam = configuration.get("module");

@darranl What's your feeling about this? Would this configuration.get("module") always be in a canonicalized form? I've tried by finding the usages of the enclosing method, but it returns that there are no usages at all, so not sure from where this method gets invoked.

[darranl]

Unless I am misreading this comment this sounds like a breaking change we can not make to the management model.

Is the call to parseCanonicalModuleIdentifier more permissive than ModuleIdentifierUtil.MODULE_NAME_CORRECTOR?

[yersan]

@darranl The ModuleIdentifierUtil.MODULE_NAME_CORRECTOR as an attribute corrector just moves the #parseCanonicalModuleIdentifier(String) to the attribute, so it will always return the canonicalized form of the module, see

wildfly-core/controller/src/main/java/org/jboss/as/controller/ModuleIdentifierUtil.java

Lines 94 to 97 in b5ab648

	/**
	* A {@link ParameterCorrector} that {@link #parseCanonicalModuleIdentifier(String) canonicalizes}
	* values that are meant to represent JBoss Modules module names.
	*/

.

They are equivalents, but having it at the attribute level is preferred since it is supposed to be the entry of the user values.

[yersan]

The idea has always been to place this sort of conversions/adjustments as close as possible to the external inputs, hence on the attribute definitions via ModuleIdentifierUtil.MODULE_NAME_CORRECTOR, however, for this JaasCustomSecurityRealmWrapper.java class, which is indeed deprecated, I'm not sure what the external input is for this configuration object

[darranl]

This sounds at risk of being a breaking model change.

I think I would like to see test cases that demonstrate that working values that can be specified today will continue to work without error even where we previously relied on passing them through parseCanonicalModuleIdentifier

[darranl]

The part I would still like to confirm is, "Is there a chance that this is going to break previously valid configurations causing those to have model time validation errors as they migrate to the WildFly version that contains this?"

[darranl]

Can we guarantee that all existing configurations will still pass even if different formats may have been used in them?

Should we maybe have a test case to demonstrate that this does not break backwards compatibility?

[github-actions]

There has been no activity on this PR for 45 days. It will be auto-closed after 90 days.