Skip to content

Commit

Permalink
[WFLY-9176] Don't clone the entire resource tree to check for the pre…
Browse files Browse the repository at this point in the history
…sence of another subsystem resource

TODO Why not use capabilities for this?
  • Loading branch information
bstansberry committed Aug 5, 2017
1 parent 4d34cb7 commit 13fef10
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
Expand Up @@ -67,9 +67,9 @@ public void installServices(OperationContext context, ModelNode model) throws Op
// Handle case where ejb subsystem has already installed services for this cache-container
// This can happen if the ejb cache-container is added to a running server
if (context.getProcessType().isServer() && !context.isBooting() && name.equals("ejb")) {
Resource rootResource = context.readResourceFromRoot(PathAddress.EMPTY_ADDRESS);
PathElement ejbPath = PathElement.pathElement(ModelDescriptionConstants.SUBSYSTEM, "ejb3");
if (rootResource.hasChild(ejbPath) && rootResource.getChild(ejbPath).hasChild(PathElement.pathElement("service", "remote"))) {
Resource ejbResource = safeGetResource(context, ejbPath);
if (ejbResource != null && ejbResource.hasChild(PathElement.pathElement("service", "remote"))) {
// Following restart, these services will be installed by this handler, rather than the ejb remote handler
context.addStep(new OperationStepHandler() {
@Override
Expand Down Expand Up @@ -139,4 +139,13 @@ public void removeServices(OperationContext context, ModelNode model) throws Ope

EnumSet.allOf(CacheContainerResourceDefinition.Capability.class).stream().map(component -> component.getServiceName(address)).forEach(serviceName -> context.removeService(serviceName));
}

private static Resource safeGetResource(OperationContext context, PathElement path) {
try {
return context.readResourceFromRoot(PathAddress.pathAddress(path), false);
} catch (RuntimeException e) {
// No such resource
return null;
}
}
}
Expand Up @@ -92,9 +92,9 @@ void installRuntimeServices(final OperationContext context, final ModelNode mode
new ClientMappingsRegistryBuilder(clientMappingsClusterName).configure(context).build(target).setInitialMode(ServiceController.Mode.ON_DEMAND).install();

// Handle case where no infinispan subsystem exists or does not define an ejb cache-container
Resource rootResource = context.readResourceFromRoot(PathAddress.EMPTY_ADDRESS);
PathElement infinispanPath = PathElement.pathElement(ModelDescriptionConstants.SUBSYSTEM, "infinispan");
if (!rootResource.hasChild(infinispanPath) || !rootResource.getChild(infinispanPath).hasChild(PathElement.pathElement("cache-container", clientMappingsClusterName))) {
Resource infinispanResource = safeGetResource(context, infinispanPath);
if (infinispanResource == null || !infinispanResource.hasChild(PathElement.pathElement("cache-container", clientMappingsClusterName))) {
// Install services that would normally be installed by this container/cache
CapabilityServiceSupport support = context.getCapabilityServiceSupport();
for (GroupBuilderProvider provider : ServiceLoader.load(LocalGroupBuilderProvider.class, LocalGroupBuilderProvider.class.getClassLoader())) {
Expand Down Expand Up @@ -167,4 +167,13 @@ private String getClassNameForChannelOptionType(final String optionType) {
}
throw EjbLogger.ROOT_LOGGER.unknownChannelCreationOptionType(optionType);
}

private static Resource safeGetResource(OperationContext context, PathElement path) {
try {
return context.readResourceFromRoot(PathAddress.pathAddress(path), false);
} catch (RuntimeException e) {
// No such resource
return null;
}
}
}

3 comments on commit 13fef10

@pferraro
Copy link
Contributor

Choose a reason for hiding this comment

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

@bstansberry These two blocks of code only exist to maintain configuration compatibility with AS 7.2. I intend to remove them completely once this compatibility constraint is lifted. The requisite capability references are already in place.

@bstansberry
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pferraro This in general is a good topic for the eap-pm list or some other discussion with PM. AS 7.2 became EAP 6.1. We only require mixed-version domains to work for 6.2 or later slaves. We've wanted general API compatibility beyond that but as time passes it's worth considering dropping really old things so we can prune code.

@pferraro
Copy link
Contributor

Choose a reason for hiding this comment

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

@bstansberry By AS 7.2, I mean the release stream that would continue through EAP 6.4. To be specific, without these blocks of code, the default configuration shipped with EAP 6.4 would fail model validation due to missing required capabilities. So this has less to do with mixed domains, and more to do with validity of legacy configuration defaults. I'll file a JBEAP jira to track this.

Please sign in to comment.