Skip to content

Commit

Permalink
W-17475148/W-17484538: Fix NPE in MapEntryBeanDefinitionCreator/MapBe…
Browse files Browse the repository at this point in the history 
…anDefinitionCreator (#14097) (#14102)

* W-17475148: Simple type params handler not handling certain expression param values
* W-17484538: Disambiguate colliding componentIds in ComponentBuildingDefinitionRegistry
  • Loading branch information
@elrodro83
elrodro83 authored Dec 23, 2024
1 parent 0f154e3 commit c6824b8
Show file tree
Hide file tree
Showing 12 changed files with 320 additions and 23 deletions.
14 changes: 14 additions & 0 deletions modules/spring-config/api-changes.json
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
{
"4.6.11": {
"revapi": {
"differences": {
"differences": [
{
"ignore": true,
"code": "java.method.added",
"new": "method java.util.Optional<org.mule.runtime.dsl.api.component.ComponentBuildingDefinition<?>> org.mule.runtime.config.api.dsl.model.ComponentBuildingDefinitionRegistry::getBuildingDefinition(org.mule.runtime.api.component.ComponentIdentifier, java.util.function.Predicate<org.mule.runtime.dsl.api.component.ComponentBuildingDefinition<?>>)",
"justification": "W-17484538: Disambiguate colliding componentIds in ComponentBuildingDefinitionRegistry"
}
]
}
}
},
"4.5.0": {
"revapi": {
"ignore": [
Expand Down
45 changes: 40 additions & 5 deletions .../main/java/org/mule/runtime/config/api/dsl/model/ComponentBuildingDefinitionRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,31 @@
*/
package org.mule.runtime.config.api.dsl.model;

import static java.util.Optional.ofNullable;
import static org.mule.runtime.config.api.dsl.model.ComponentBuildingDefinitionRegistry.WrapperElementType.COLLECTION;
import static org.mule.runtime.config.api.dsl.model.ComponentBuildingDefinitionRegistry.WrapperElementType.MAP;
import static org.mule.runtime.config.api.dsl.model.ComponentBuildingDefinitionRegistry.WrapperElementType.SINGLE;
import static org.mule.runtime.internal.dsl.DslConstants.CORE_PREFIX;

import static java.util.Collections.emptyList;
import static java.util.Optional.empty;
import static java.util.Optional.ofNullable;

import org.mule.runtime.api.component.ComponentIdentifier;
import org.mule.runtime.config.api.dsl.processor.AbstractAttributeDefinitionVisitor;
import org.mule.runtime.dsl.api.component.AttributeDefinition;
import org.mule.runtime.dsl.api.component.ComponentBuildingDefinition;
import org.mule.runtime.dsl.api.component.ComponentBuildingDefinitionProvider;
import org.mule.runtime.dsl.api.component.KeyAttributeDefinitionPair;
import org.mule.runtime.dsl.api.component.SetterAttributeDefinition;

import java.util.ArrayDeque;
import java.util.Collection;
import java.util.Deque;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Predicate;

/**
* Registry with all {@link ComponentBuildingDefinition} that where discovered in the classpath.
Expand All @@ -35,7 +43,7 @@
@Deprecated
public final class ComponentBuildingDefinitionRegistry {

private final Map<ComponentIdentifier, ComponentBuildingDefinition<?>> builderDefinitionsMap = new HashMap<>();
private final Map<ComponentIdentifier, Deque<ComponentBuildingDefinition<?>>> builderDefinitionsMap = new HashMap<>();
private final Map<String, WrapperElementType> wrapperIdentifierAndTypeMap = new HashMap<>();

/**
Expand All @@ -44,7 +52,11 @@ public final class ComponentBuildingDefinitionRegistry {
* @param builderDefinition definition to be added in the registry
*/
public void register(ComponentBuildingDefinition<?> builderDefinition) {
builderDefinitionsMap.put(builderDefinition.getComponentIdentifier(), builderDefinition);
builderDefinitionsMap.computeIfAbsent(builderDefinition.getComponentIdentifier(),
// Use a stack structure so the order is consistent across executions
// and keep the behavior that the last element to be added takes precedence
k -> new ArrayDeque<>())
.push(builderDefinition);
wrapperIdentifierAndTypeMap.putAll(getWrapperIdentifierAndTypeMap(builderDefinition));
}

Expand All @@ -55,7 +67,30 @@ public void register(ComponentBuildingDefinition<?> builderDefinition) {
* @return the definition to build the component
*/
public Optional<ComponentBuildingDefinition<?>> getBuildingDefinition(ComponentIdentifier identifier) {
return ofNullable(builderDefinitionsMap.get(identifier));
final Deque<ComponentBuildingDefinition<?>> definitions = builderDefinitionsMap.get(identifier);
return definitions == null
? empty()
: ofNullable(definitions.peek());
}

/**
* Lookups a {@code ComponentBuildingDefinition} for a certain configuration component and a certain condition.
*
* @param identifier the component identifier
* @param condition how to determine which of the available definitions to use
* @return the definition to build the component
*/
public Optional<ComponentBuildingDefinition<?>> getBuildingDefinition(ComponentIdentifier identifier,
Predicate<ComponentBuildingDefinition<?>> condition) {
Collection<ComponentBuildingDefinition<?>> buildingDefinitions = builderDefinitionsMap.get(identifier);
if (buildingDefinitions == null) {
buildingDefinitions = emptyList();
}

return buildingDefinitions
.stream()
.filter(condition)
.findFirst();
}

/**
Expand Down Expand Up @@ -109,7 +144,7 @@ public void onMultipleValues(KeyAttributeDefinitionPair[] definitions) {
Consumer<AttributeDefinition> collectWrappersConsumer =
attributeDefinition -> attributeDefinition.accept(wrapperIdentifiersCollector);
buildingDefinition.getSetterParameterDefinitions().stream()
.map(setterAttributeDefinition -> setterAttributeDefinition.getAttributeDefinition())
.map(SetterAttributeDefinition::getAttributeDefinition)
.forEach(collectWrappersConsumer);
buildingDefinition.getConstructorAttributeDefinition().stream().forEach(collectWrappersConsumer);
return wrapperIdentifierAndTypeMap;
Expand Down
11 changes: 9 additions & 2 deletions ...nfig/src/main/java/org/mule/runtime/config/internal/dsl/spring/BeanDefinitionCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
*/
package org.mule.runtime.config.internal.dsl.spring;

import static org.mule.runtime.api.i18n.I18nMessageFactory.createStaticMessage;

import static org.springframework.beans.factory.support.BeanDefinitionBuilder.genericBeanDefinition;

import org.mule.runtime.api.exception.MuleRuntimeException;
import org.mule.runtime.ast.api.ComponentAst;
import org.mule.runtime.config.internal.dsl.model.SpringComponentModel;
import org.mule.runtime.config.internal.factories.ConstantFactoryBean;
Expand Down Expand Up @@ -41,8 +44,12 @@ public void setNext(BeanDefinitionCreator<R> nextBeanDefinitionCreator) {
* @param request
*/
public final void processRequest(Map<ComponentAst, SpringComponentModel> springComponentModels, R request) {
if (handleRequest(springComponentModels, request)) {
return;
try {
if (handleRequest(springComponentModels, request)) {
return;
}
} catch (Exception e) {
throw new MuleRuntimeException(createStaticMessage("Exception processing " + request.toString()), e);
}
if (next != null) {
next.processRequest(springComponentModels, request);
Expand Down
52 changes: 42 additions & 10 deletions ...nfig/src/main/java/org/mule/runtime/config/internal/dsl/spring/BeanDefinitionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
import org.mule.runtime.config.internal.context.SpringConfigurationComponentLocator;
import org.mule.runtime.config.internal.dsl.model.SpringComponentModel;
import org.mule.runtime.dsl.api.component.ComponentBuildingDefinition;
import org.mule.runtime.dsl.api.component.TypeDefinition.MapEntryType;
import org.mule.runtime.dsl.api.component.TypeDefinitionVisitor;
import org.mule.runtime.extension.api.property.NoWrapperModelProperty;

import java.util.ArrayList;
Expand Down Expand Up @@ -205,7 +207,8 @@ public void resolveComponent(Map<ComponentAst, SpringComponentModel> springCompo
resolveComponentBeanDefinition(springComponentModels, componentHierarchy, component,
paramsModels, registry, componentLocator,
nestedComp -> resolveComponent(springComponentModels, nestedHierarchy, nestedComp,
registry, componentLocator));
registry, componentLocator),
false);
}

private List<SpringComponentModel> resolveParameterGroup(Map<ComponentAst, SpringComponentModel> springComponentModels,
Expand Down Expand Up @@ -272,20 +275,21 @@ private List<SpringComponentModel> resolveParamBeanDefinitionFixedValue(Map<Comp
AtomicReference<SpringComponentModel> model = new AtomicReference<>();
param.getModel().getType().accept(new MetadataTypeVisitor() {

protected void visitMultipleChildren(List<Object> values) {
protected void visitMultipleChildren(List<Object> values, boolean mapType) {
final List<ComponentAst> updatedHierarchy = new ArrayList<>(componentHierarchy);
updatedHierarchy.add(paramOwnerComponent);

if (values != null) {
values.stream()
.filter(child -> child instanceof ComponentAst)
.filter(ComponentAst.class::isInstance)
.forEach(child -> resolveComponentBeanDefinition(springComponentModels, updatedHierarchy, (ComponentAst) child,
emptyList(),
registry, componentLocator,
nestedComp -> resolveComponent(springComponentModels,
updatedHierarchy,
nestedComp, registry,
componentLocator)));
componentLocator),
mapType));
}

resolveComponentBeanDefinitionComplexParam(springComponentModels, updatedHierarchy, paramOwnerComponent, param,
Expand All @@ -299,7 +303,7 @@ protected void visitMultipleChildren(List<Object> values) {
public void visitArrayType(ArrayType arrayType) {
final Object complexValue = param.getValue().getRight();
if (complexValue instanceof List) {
visitMultipleChildren((List) complexValue);
visitMultipleChildren((List) complexValue, false);
} else {
// references to a list defined elsewhere
resolveParamBeanDefinitionSimpleType(springComponentModels, componentHierarchy, paramOwnerComponent, param, registry,
Expand All @@ -314,7 +318,7 @@ public void visitObject(ObjectType objectType) {

if (isMap(objectType)) {
if (complexValue instanceof List) {
visitMultipleChildren((List) complexValue);
visitMultipleChildren((List) complexValue, true);
} else {
// references to a map defined elsewhere
resolveParamBeanDefinitionSimpleType(springComponentModels, componentHierarchy, paramOwnerComponent, param, registry,
Expand All @@ -329,7 +333,6 @@ public void visitObject(ObjectType objectType) {

if (complexValue instanceof ComponentAst) {
final ComponentAst child = (ComponentAst) complexValue;

List<SpringComponentModel> childParamsModels = new ArrayList<>();

child.getModel(ParameterizedModel.class)
Expand Down Expand Up @@ -400,13 +403,19 @@ private Optional<SpringComponentModel> resolveComponentBeanDefinition(Map<Compon
List<SpringComponentModel> paramsModels,
BeanDefinitionRegistry registry,
SpringConfigurationComponentLocator componentLocator,
Consumer<ComponentAst> nestedComponentParamProcessor) {
Consumer<ComponentAst> nestedComponentParamProcessor,
boolean parentMapType) {
Optional<ComponentBuildingDefinition<?>> buildingDefinitionOptional =
componentBuildingDefinitionRegistry.getBuildingDefinition(component.getIdentifier());
componentBuildingDefinitionRegistry.getBuildingDefinition(component.getIdentifier(), bd -> {
final IsMapEntryTypeVisitor isMapEntryTypeVisitor = new IsMapEntryTypeVisitor();
bd.getTypeDefinition().visit(isMapEntryTypeVisitor);
return parentMapType == isMapEntryTypeVisitor.isMapType();
});
if (buildingDefinitionOptional.isPresent() || customBuildersComponentIdentifiers.contains(component.getIdentifier())) {
final CreateComponentBeanDefinitionRequest request =
new CreateComponentBeanDefinitionRequest(componentHierarchy, component, paramsModels,
buildingDefinitionOptional.orElse(null), nestedComponentParamProcessor);
buildingDefinitionOptional.orElse(null), nestedComponentParamProcessor,
parentMapType);

this.componentProcessor.processRequest(springComponentModels, request);
handleSpringComponentModel(request.getSpringComponentModel(), springComponentModels, registry, componentLocator);
Expand All @@ -416,6 +425,29 @@ private Optional<SpringComponentModel> resolveComponentBeanDefinition(Map<Compon
}
}

private static final class IsMapEntryTypeVisitor implements TypeDefinitionVisitor {

private boolean mapType;

@Override
public void onType(Class<?> type) {}

@Override
public void onMapType(MapEntryType mapEntryType) {
this.mapType = true;
}

@Override
public void onConfigurationAttribute(String groupName, String attributeName, Class<?> enforcedClass) {}

@Override
public void onConfigurationAttribute(String attributeName, Class<?> enforcedClass) {}

public boolean isMapType() {
return mapType;
}
}

private Optional<SpringComponentModel> resolveComponentBeanDefinitionComplexParam(Map<ComponentAst, SpringComponentModel> springComponentModels,
List<ComponentAst> componentHierarchy,
ComponentAst paramOwnerComponent,
Expand Down
14 changes: 13 additions & 1 deletion ...ava/org/mule/runtime/config/internal/dsl/spring/CreateComponentBeanDefinitionRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@
public class CreateComponentBeanDefinitionRequest extends CreateBeanDefinitionRequest {

private final Consumer<ComponentAst> nestedComponentParamProcessor;
private final boolean parentMapType;

public CreateComponentBeanDefinitionRequest(List<ComponentAst> componentHierarchy,
ComponentAst component,
List<SpringComponentModel> paramsModels,
ComponentBuildingDefinition componentBuildingDefinition,
Consumer<ComponentAst> nestedComponentParamProcessor) {
Consumer<ComponentAst> nestedComponentParamProcessor,
boolean parentMapType) {
super(componentHierarchy, component, paramsModels, componentBuildingDefinition, component.getIdentifier());
this.nestedComponentParamProcessor = nestedComponentParamProcessor;
this.parentMapType = parentMapType;
}

@Override
Expand All @@ -34,4 +37,13 @@ public ComponentAst resolveConfigurationComponent() {
public Consumer<ComponentAst> getNestedComponentParamProcessor() {
return nestedComponentParamProcessor;
}

public boolean isParentMapType() {
return parentMapType;
}

@Override
public String toString() {
return "component request for `" + getComponent().toString();
}
}
7 changes: 7 additions & 0 deletions ...org/mule/runtime/config/internal/dsl/spring/CreateDslParamGroupBeanDefinitionRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,11 @@ public ParameterGroupModel getParamGroupModel() {
public ComponentParameterAst getParameter(String parameterName) {
return paramOwnerComponent.getParameter(paramGroupModel.getName(), parameterName);
}

@Override
public String toString() {
return "DSL param group request for `"
+ getParamGroupModel().getName() + "` in component `"
+ getParamOwnerComponent().toString();
}
}
8 changes: 8 additions & 0 deletions ...in/java/org/mule/runtime/config/internal/dsl/spring/CreateParamBeanDefinitionRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,12 @@ public ComponentParameterAst getParameter(String parameterName) {
.findFirst()
.orElse(null);
}

@Override
public String toString() {
return "param request for `"
+ getParam().getGroupModel().getName() + "."
+ getParam().getModel().getName() + "` in component `"
+ getParamOwnerComponent().toString();
}
}
9 changes: 8 additions & 1 deletion .../main/java/org/mule/runtime/config/internal/dsl/spring/MapEntryBeanDefinitionCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
*/
package org.mule.runtime.config.internal.dsl.spring;

import static java.util.stream.Collectors.toCollection;
import static org.mule.runtime.api.meta.model.parameter.ParameterGroupModel.DEFAULT_GROUP_NAME;
import static org.mule.runtime.config.internal.dsl.processor.ObjectTypeVisitor.DEFAULT_COLLECTION_TYPE;
import static org.mule.runtime.dsl.api.component.DslSimpleType.SIMPLE_TYPE_VALUE_PARAMETER_NAME;

import static java.util.stream.Collectors.toCollection;

import static org.springframework.beans.factory.support.BeanDefinitionBuilder.genericBeanDefinition;

import org.mule.runtime.ast.api.ComponentAst;
Expand Down Expand Up @@ -61,11 +63,16 @@ class MapEntryBeanDefinitionCreator extends BeanDefinitionCreator<CreateComponen
@Override
boolean handleRequest(Map<ComponentAst, SpringComponentModel> springComponentModels,
CreateComponentBeanDefinitionRequest request) {
if (!request.isParentMapType()) {
return false;
}

ComponentAst component = request.getComponent();
Class<?> type = request.getSpringComponentModel().getType();
if (!(MapEntryType.class.isAssignableFrom(type))) {
return false;
}

ComponentBuildingDefinition componentBuildingDefinition = request.getComponentBuildingDefinition();
request.getSpringComponentModel().setType(type);
final String key = component.getParameter(DEFAULT_GROUP_NAME, ENTRY_TYPE_KEY_PARAMETER_NAME).getResolvedRawValue();
Expand Down
14 changes: 10 additions & 4 deletions ...java/org/mule/runtime/config/internal/dsl/spring/SimpleTypeBeanBaseDefinitionCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,18 @@ protected final boolean handleRequest(Map<ComponentAst, SpringComponentModel> sp
R createBeanDefinitionRequest) {
Class<?> type = createBeanDefinitionRequest.getSpringComponentModel().getType();

if (!isSimpleType(type)) {
return false;
if (isSimpleType(type)
// Expressions are String, which are simple values for the spring bean definitions
|| isExpressionValue(createBeanDefinitionRequest)) {
createBeanDefinitionRequest.getSpringComponentModel().setType(type);
return doHandleRequest(createBeanDefinitionRequest, type);
}

createBeanDefinitionRequest.getSpringComponentModel().setType(type);
return doHandleRequest(createBeanDefinitionRequest, type);
return false;
}

protected boolean isExpressionValue(R request) {
return false;
}

protected abstract boolean doHandleRequest(R createBeanDefinitionRequest, Class<?> type);
Expand Down
5 changes: 5 additions & 0 deletions ...ava/org/mule/runtime/config/internal/dsl/spring/SimpleTypeBeanParamDefinitionCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ protected boolean doHandleRequest(CreateParamBeanDefinitionRequest createBeanDef
return true;
}

@Override
protected boolean isExpressionValue(CreateParamBeanDefinitionRequest request) {
return request.getParam().getValue().isLeft();
}

static Object resolveParamValue(final ComponentParameterAst param, boolean disableTrimWhitespaces,
boolean disablePojoCdataTrimWhitespaces) {
return param.getValue()
Expand Down
Loading

0 comments on commit c6824b8

Please sign in to comment.