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

[WFCORE-6741] [WFCORE-6652] Improve user experience when using extension or namespace of unsupported stability. #5899

Merged
merged 3 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,24 @@ static void initializeExtension(ExtensionRegistry extensionRegistry, String modu
}
while (extensions.hasNext()) {
Extension extension = extensions.next();
if (extensionRegistry.enables(extension)) {
ClassLoader oldTccl = WildFlySecurityManager.setCurrentContextClassLoaderPrivileged(extension.getClass());
try {
if (unknownModule || !extensionRegistry.getExtensionModuleNames().contains(module)) {
// This extension wasn't handled by the standalone.xml or domain.xml parsing logic, so we
// need to initialize its parsers so we can display what XML namespaces it supports
extensionRegistry.initializeParsers(extension, module, null);
// AS7-6190 - ensure we initialize parsers for other extensions from this module
// now that we know the registry was unaware of the module
unknownModule = true;
}
ClassLoader oldTccl = WildFlySecurityManager.setCurrentContextClassLoaderPrivileged(extension.getClass());
try {
if (unknownModule || !extensionRegistry.getExtensionModuleNames().contains(module)) {
// This extension wasn't handled by the standalone.xml or domain.xml parsing logic, so we
// need to initialize its parsers so we can display what XML namespaces it supports
extensionRegistry.initializeParsers(extension, module, null);
// AS7-6190 - ensure we initialize parsers for other extensions from this module
// now that we know the registry was unaware of the module
unknownModule = true;
}
// If extension is not enabled by stability level of process, skip model registration
if (extensionRegistry.enables(extension)) {
extension.initialize(extensionRegistry.getExtensionContext(module, extension.getStability(), rootRegistration, extensionRegistryType));
} finally {
WildFlySecurityManager.setCurrentContextClassLoaderPrivileged(oldTccl);
} else {
ControllerLogger.ROOT_LOGGER.unstableExtension(extension.getClass().getName(), module);
}
} finally {
WildFlySecurityManager.setCurrentContextClassLoaderPrivileged(oldTccl);
}
}
} catch (ModuleNotFoundException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,9 @@ public ExtensionParsingContext getExtensionParsingContext(final String moduleNam
* be any actual parsing (e.g. in a slave Host Controller or in a server in a managed domain)
*/
public void initializeParsers(final Extension extension, final String moduleName, final XMLMapper xmlMapper) {
ExtensionParsingContextImpl parsingContext = new ExtensionParsingContextImpl(moduleName, xmlMapper);
extension.initializeParsers(parsingContext);
parsingContext.attemptCurrentParserInitialization();
try (AbstractExtensionParsingContext context = this.enables(extension) ? new ExtensionParsingContextImpl(moduleName, xmlMapper) : new UnstableExtensionParsingContext(moduleName, xmlMapper)) {
extension.initializeParsers(context);
}
Comment on lines +351 to +353
Copy link
Collaborator

Choose a reason for hiding this comment

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

Autocloseable interface is used to mark objects that could hold resources, here it is used to invoke the ExtensionParsingContext#attemptCurrentParserInitialization(), since the abstract class does nothing on its close() method.

Is that usage correct? It looks inappropriate. I don't see where ExtensionParsingContext#attemptCurrentParserInitialization() was trying to release resources that can justify the movement to an Autocloseable.close() implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • An extension parsing context could hold resources.
  • The JDK itself implements AutoCloseable for objects that do not "hold resources" in the traditional sense, but that have a defined lifecycle, e.g. DoubleStream, IntStream, LongStream, Stream, etc.
  • The usage here is an internal implementation detail of the private class and is not publicly exposed.

}

/**
Expand Down Expand Up @@ -542,30 +542,25 @@ public Stability getStability() {
return this.stability;
}

private class ExtensionParsingContextImpl implements ExtensionParsingContext {

private final ExtensionInfo extension;
private boolean hasNonSupplierParser;
private String latestNamespace;
private Supplier<XMLElementReader<List<ModelNode>>> latestSupplier;
private abstract class AbstractExtensionParsingContext implements ExtensionParsingContext, AutoCloseable {
final ExtensionInfo extension;

private ExtensionParsingContextImpl(String extensionName, XMLMapper xmlMapper) {
extension = getExtensionInfo(extensionName);
AbstractExtensionParsingContext(String extensionName, XMLMapper xmlMapper) {
this.extension = ExtensionRegistry.this.getExtensionInfo(extensionName);
if (xmlMapper != null) {
synchronized (extension) {
extension.xmlMapper = xmlMapper;
synchronized (this.extension) {
this.extension.xmlMapper = xmlMapper;
}
}
}

@Override
public ProcessType getProcessType() {
return processType;
return ExtensionRegistry.this.processType;
}

@Override
public RunningMode getRunningMode() {
return runningModeControl.getRunningMode();
return ExtensionRegistry.this.runningModeControl.getRunningMode();
}

@Override
Expand All @@ -578,6 +573,22 @@ public <F extends Feature> boolean enables(F feature) {
return ExtensionRegistry.this.enables(feature);
}

@Override
public void close() {
// Do nothing
}
}

private class ExtensionParsingContextImpl extends AbstractExtensionParsingContext {

private boolean hasNonSupplierParser;
private String latestNamespace;
private Supplier<XMLElementReader<List<ModelNode>>> latestSupplier;

ExtensionParsingContextImpl(String extensionName, XMLMapper xmlMapper) {
super(extensionName, xmlMapper);
}

@Override
public void setSubsystemXmlMapping(String subsystemName, String namespaceUri, XMLElementReader<List<ModelNode>> reader) {
assert subsystemName != null : "subsystemName is null";
Expand Down Expand Up @@ -618,7 +629,8 @@ public void setProfileParsingCompletionHandler(ProfileParsingCompletionHandler h
}
}

private void attemptCurrentParserInitialization() {
@Override
public void close() {
if (ExtensionRegistry.this.processType != ProcessType.DOMAIN_SERVER
&& !hasNonSupplierParser && latestSupplier != null) {
// We've only been passed suppliers for parsers, which means the model
Expand All @@ -645,6 +657,37 @@ private void preCacheParserDescription(XMLElementReader<List<ModelNode>> reader)
}
}

private class UnstableExtensionParsingContext extends AbstractExtensionParsingContext {

UnstableExtensionParsingContext(String extensionName, XMLMapper xmlMapper) {
super(extensionName, xmlMapper);
}

@Override
public void setSubsystemXmlMapping(String subsystemName, String namespaceUri, XMLElementReader<List<ModelNode>> reader) {
this.setSubsystemXMLMapping(subsystemName, namespaceUri);
}

@Override
public void setSubsystemXmlMapping(String subsystemName, String namespaceUri, Supplier<XMLElementReader<List<ModelNode>>> supplier) {
this.setSubsystemXMLMapping(subsystemName, namespaceUri);
}

private void setSubsystemXMLMapping(String subsystemName, String namespaceUri) {
synchronized (this.extension) {
this.extension.getSubsystemInfo(subsystemName).addParsingNamespace(namespaceUri);
if (this.extension.xmlMapper != null) {
this.extension.xmlMapper.registerRootElement(new QName(namespaceUri, SUBSYSTEM), new UnstableSubsystemNamespaceParser(subsystemName));
}
}
}

@Override
public void setProfileParsingCompletionHandler(ProfileParsingCompletionHandler handler) {
// Ignore
}
}

@SuppressWarnings("deprecation")
private class ExtensionContextImpl implements ExtensionContext, ExtensionContextSupplement {

Expand Down Expand Up @@ -733,7 +776,7 @@ public ProcessType getProcessType() {

@Override
public Stability getStability() {
return this.profileRegistration.getStability();
return ExtensionRegistry.this.getStability();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright The WildFly Authors
* SPDX-License-Identifier: Apache-2.0
*/

package org.jboss.as.controller.extension;

import java.util.List;

import javax.xml.stream.XMLStreamException;

import org.jboss.as.controller.logging.ControllerLogger;
import org.jboss.dmr.ModelNode;
import org.jboss.staxmapper.XMLElementReader;
import org.jboss.staxmapper.XMLExtendedStreamReader;

/**
* A placeholder parser for a subsystem namespace not enabled by the stability level of the current process.
*/
public class UnstableSubsystemNamespaceParser implements XMLElementReader<List<ModelNode>> {
private final String subsystemName;

public UnstableSubsystemNamespaceParser(String subsystemName) {
this.subsystemName = subsystemName;
}

@Override
public void readElement(XMLExtendedStreamReader reader, List<ModelNode> value) throws XMLStreamException {
throw ControllerLogger.ROOT_LOGGER.unstableSubsystemNamespace(this.subsystemName, reader.getNamespaceURI());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3752,4 +3752,11 @@ OperationFailedRuntimeException capabilityAlreadyRegisteredInContext(String capa

@Message(id = 504, value = "The operation %s is not defined for resource %s.")
UnsupportedOperationException missingOperationForResource(String op, String address);

@Message(id = 505, value = "%s subsystem namespace %s is not enabled by the current stability level")
XMLStreamException unstableSubsystemNamespace(String subsystemName, String namespaceURI);

@LogMessage(level = WARN)
@Message(id = 506, value = "Extension %s from module %s is not enabled by the current stability level")
void unstableExtension(String extensionName, String moduleName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.jboss.as.controller.ProcessType;
import org.jboss.as.controller.RunningMode;
import org.jboss.as.controller.SubsystemSchema;
import org.jboss.as.controller.extension.UnstableSubsystemNamespaceParser;
import org.jboss.dmr.ModelNode;
import org.jboss.staxmapper.XMLElementReader;

Expand Down Expand Up @@ -81,9 +82,8 @@ public interface ExtensionParsingContext extends FeatureRegistry {
*/
default <S extends SubsystemSchema<S>> void setSubsystemXmlMappings(String subsystemName, Set<S> schemas) {
for (S schema : schemas) {
if (this.enables(schema)) {
this.setSubsystemXmlMapping(subsystemName, schema.getNamespace().getUri(), schema);
}
XMLElementReader<List<ModelNode>> reader = this.enables(schema) ? schema : new UnstableSubsystemNamespaceParser(subsystemName);
this.setSubsystemXmlMapping(subsystemName, schema.getNamespace().getUri(), reader);
}
}

Expand Down
Loading