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.

Sign up for GitHub

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

W-17463948: Prevent double injection of objects in registry #14066

Merged
merged 8 commits into from
Dec 18, 2024
Merged

W-17463948: Prevent double injection of objects in registry #14066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b4025e3
Prevent double injection of objects in registry
elrodro83 Dec 9, 2024
467af7f
constant FB
elrodro83 Dec 11, 2024
e233731
fix
elrodro83 Dec 12, 2024
6067e46
piggyback
elrodro83 Dec 13, 2024
002b5e0
wrapup
elrodro83 Dec 13, 2024
cdefa22
rebased
elrodro83 Dec 18, 2024
4bcd8f8
self review
elrodro83 Dec 18, 2024
513ff1f
Merge branch 'master' into chore/double-inject
elrodro83 Dec 18, 2024
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
20 changes: 10 additions & 10 deletions ...org/mule/runtime/core/internal/streaming/object/test/AbstractObjectStreamingTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@
*/
package org.mule.runtime.core.internal.streaming.object.test;

import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic;
import static org.apache.commons.lang3.RandomStringUtils.insecure;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.hamcrest.MatcherAssert.assertThat;

import org.mule.runtime.api.streaming.object.CursorIterator;
import org.mule.runtime.core.api.streaming.iterator.Consumer;
import org.mule.runtime.core.api.streaming.iterator.ConsumerStreamingIterator;
import org.mule.runtime.core.api.streaming.iterator.StreamingIterator;
import org.mule.tck.junit4.AbstractMuleTestCase;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -18,13 +24,7 @@
import java.util.List;
import java.util.NoSuchElementException;

import org.mule.runtime.api.streaming.object.CursorIterator;
import org.mule.runtime.core.api.streaming.iterator.Consumer;
import org.mule.runtime.core.api.streaming.iterator.ConsumerStreamingIterator;
import org.mule.runtime.core.api.streaming.iterator.StreamingIterator;
import org.mule.tck.junit4.AbstractMuleContextTestCase;

public abstract class AbstractObjectStreamingTestCase extends AbstractMuleContextTestCase {
public abstract class AbstractObjectStreamingTestCase extends AbstractMuleTestCase {

protected final List<Object> data;

Expand All @@ -36,7 +36,7 @@ public AbstractObjectStreamingTestCase(int dataSize) {
}

protected Object createDataInstance() {
return randomAlphabetic(10);
return insecure().nextAlphabetic(10);
}

protected <T> StreamingIterator<T> toStreamingIterator(List<T> data) {
Expand Down
2 changes: 1 addition & 1 deletion .../org/mule/runtime/core/internal/streaming/object/test/CursorIteratorProviderTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.hamcrest.MatcherAssert.assertThat;

import org.mule.runtime.api.streaming.exception.StreamingBufferSizeExceededException;
import org.mule.runtime.api.streaming.object.CursorIterator;
Expand Down
25 changes: 16 additions & 9 deletions ...c/test/java/org/mule/runtime/core/internal/util/store/MuleObjectStoreManagerTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@
*/
package org.mule.runtime.core.internal.util.store;

import static org.mule.runtime.api.scheduler.SchedulerConfig.config;
import static org.mule.runtime.api.store.ObjectStoreManager.BASE_IN_MEMORY_OBJECT_STORE_KEY;
import static org.mule.runtime.api.store.ObjectStoreManager.BASE_PERSISTENT_OBJECT_STORE_KEY;

import static java.lang.Thread.currentThread;
import static java.util.Optional.of;

import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mule.runtime.api.scheduler.SchedulerConfig.config;
import static org.mule.runtime.api.store.ObjectStoreManager.BASE_IN_MEMORY_OBJECT_STORE_KEY;
import static org.mule.runtime.api.store.ObjectStoreManager.BASE_PERSISTENT_OBJECT_STORE_KEY;

import io.qameta.allure.Issue;
import org.mule.runtime.api.artifact.Registry;
import org.mule.runtime.api.exception.MuleException;
import org.mule.runtime.api.lifecycle.InitialisationException;
Expand All @@ -38,16 +39,18 @@
import org.mule.tck.probe.Probe;
import org.mule.tck.size.SmallTest;

import java.io.Serializable;
import java.util.NoSuchElementException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import java.io.Serializable;
import java.util.NoSuchElementException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import io.qameta.allure.Issue;

@SmallTest
public class MuleObjectStoreManagerTestCase extends AbstractMuleTestCase {
Expand All @@ -74,6 +77,10 @@ public void setup() {
schedulerService = new SimpleUnitTestSupportSchedulerService();
muleContext = mock(MuleContextWithRegistry.class);
MuleConfiguration muleConfiguration = mock(MuleConfiguration.class);
when(muleConfiguration.getWorkingDirectory()).thenReturn(tempWorkDir.getRoot().getAbsolutePath());
when(muleContext.getConfiguration()).thenReturn(muleConfiguration);
when(muleContext.getExecutionClassLoader()).thenReturn(this.getClass().getClassLoader());

Registry registry = mock(Registry.class);
createRegistryAndBaseStore(muleConfiguration, new JavaObjectSerializer(this.getClass().getClassLoader()), registry);
when(muleContext.getSchedulerBaseConfig())
Expand Down
5 changes: 2 additions & 3 deletions core/src/main/java/org/mule/runtime/core/internal/lifecycle/DefaultLifecycleInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@

import static com.google.common.cache.CacheBuilder.newBuilder;

import org.mule.runtime.core.internal.lifecycle.phases.LifecyclePhase;

import java.util.Map;
import java.util.Optional;

import org.mule.runtime.core.internal.lifecycle.phases.LifecyclePhase;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -67,8 +68,6 @@ public boolean beforePhaseExecution(LifecyclePhase phase, Object object) {
}
return false;
}
}
if (isFinalPhase(phase) && (initialPhaseLifecycleClass.isAssignableFrom(object.getClass()))) {
return processedObjects.containsKey(object);
}
processedObjects.put(object, object);
Expand Down
2 changes: 1 addition & 1 deletion ...ct/activation/internal/classloader/AbstractArtifactClassLoaderConfigurationAssembler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public ClassLoaderConfiguration createClassLoaderConfiguration() {

final List<URL> dependenciesArtifactsUrls =
loadUrls(getProjectFolder(), bundleDependencies, classLoaderConfigurationBuilder);
dependenciesArtifactsUrls.forEach(classLoaderConfigurationBuilder::containing);
classLoaderConfigurationBuilder.containing(dependenciesArtifactsUrls);

if (shouldPopulateLocalPackages()) {
populateLocalPackages(classLoaderConfigurationBuilder);
Expand Down
2 changes: 0 additions & 2 deletions ...e/runtime/module/artifact/activation/internal/plugin/ArtifactPluginDescriptorFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
*/
package org.mule.runtime.module.artifact.activation.internal.plugin;

import static java.util.Optional.empty;

import org.mule.runtime.api.deployment.meta.MuleArtifactLoaderDescriptor;
import org.mule.runtime.api.deployment.meta.MulePluginModel;
import org.mule.runtime.module.artifact.activation.internal.descriptor.AbstractArtifactDescriptorFactory;
Expand Down
28 changes: 10 additions & 18 deletions ...g-support/src/test/java/org/mule/test/module/extension/values/AbstractValuesTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@
package org.mule.test.module.extension.values;

import static org.mule.runtime.api.value.ValueProviderService.VALUE_PROVIDER_SERVICE_KEY;
import static org.mule.tck.junit4.matcher.value.ValueResultSuccessMatcher.isSuccess;
import static org.mule.test.allure.AllureConstants.SdkToolingSupport.SDK_TOOLING_SUPPORT;
import static org.mule.test.allure.AllureConstants.SdkToolingSupport.ValueProvidersStory.VALUE_PROVIDERS_SERVICE;

import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toSet;

import static org.hamcrest.core.IsCollectionContaining.hasItems;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsIterableContaining.hasItems;

import org.mule.functional.junit4.MuleArtifactFunctionalTestCase;
import org.mule.runtime.api.component.location.Location;
import org.mule.runtime.api.value.ResolvingFailure;
import org.mule.runtime.api.value.Value;
import org.mule.runtime.api.value.ValueProviderService;
import org.mule.runtime.api.value.ValueResult;
Expand Down Expand Up @@ -78,24 +79,18 @@ Matcher<Iterable<Value>> hasValues(ValueMatcher... valuesMatchers) {
Set<Value> getValuesFromSource(String flowName, String parameterName) throws Exception {
ValueResult valueResult =
valueProviderService.getValues(Location.builder().globalName(flowName).addSourcePart().build(), parameterName);
if (valueResult.getFailure().isPresent()) {
ResolvingFailure resolvingFailure = valueResult.getFailure().get();
throw new ValueResolvingException(resolvingFailure.getMessage(), resolvingFailure.getFailureCode());
}
return valueResult
.getValues();

assertThat(valueResult, isSuccess());
return valueResult.getValues();
}

Set<Value> getValuesFromSource(String flowName, String parameterName, String targetSelector) throws Exception {
ValueResult valueResult =
valueProviderService.getFieldValues(Location.builder().globalName(flowName).addSourcePart().build(), parameterName,
targetSelector);
if (valueResult.getFailure().isPresent()) {
ResolvingFailure resolvingFailure = valueResult.getFailure().get();
throw new ValueResolvingException(resolvingFailure.getMessage(), resolvingFailure.getFailureCode());
}
return valueResult
.getValues();

assertThat(valueResult, isSuccess());
return valueResult.getValues();
}

Set<Value> getValues(String flowName, String parameterName) throws Exception {
Expand Down Expand Up @@ -153,10 +148,7 @@ public ValueResult getFieldValueResultFromConnection(String configName, String p
}

private Set<Value> checkResultAndRetrieveValues(ValueResult values) throws ValueResolvingException {
if (!values.isSuccess()) {
ResolvingFailure resolvingFailure = values.getFailure().get();
throw new ValueResolvingException(resolvingFailure.getMessage(), resolvingFailure.getFailureCode());
}
assertThat(values, isSuccess());
return values.getValues();
}

Expand Down
23 changes: 17 additions & 6 deletions ...rg/mule/runtime/config/internal/context/AbstractSpringMuleContextServiceConfigurator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ protected static BeanDefinition getBeanDefinition(Class<?> beanType) {
}

protected void registerConstantBeanDefinition(String serviceId, Object impl) {
registerBeanDefinition(serviceId, getConstantObjectBeanDefinition(impl));
registerConstantBeanDefinition(serviceId, impl, false);
}

protected void registerConstantBeanDefinition(String serviceId, Object impl, boolean inject) {
registerBeanDefinition(serviceId, getConstantObjectBeanDefinition(impl, inject));
}

protected void registerBeanDefinition(String serviceId, BeanDefinition beanDefinition) {
Expand All @@ -65,7 +69,7 @@ protected void registerBeanDefinition(String serviceId, BeanDefinition beanDefin
}

protected boolean isServiceRuntimeProvided(final CustomService<?> customService) {
return customService.getServiceImpl().map(impl -> impl instanceof Service).orElse(false)
return customService.getServiceImpl().map(Service.class::isInstance).orElse(false)
|| customService.getServiceClass().map(Service.class::isAssignableFrom).orElse(false);
}

Expand Down Expand Up @@ -95,13 +99,13 @@ protected BeanDefinition getCustomServiceBeanDefinition(CustomService customServ
.forApplication(new InjectParamsFromContextServiceMethodInvoker(serviceLocator));
}

beanDefinition = getConstantObjectBeanDefinition(servImpl);
beanDefinition = getConstantObjectBeanDefinition(servImpl, true);
} else {
beanDefinition =
getConstantObjectBeanDefinition(createInjectProviderParamsServiceProxy((Service) servImpl, serviceLocator));
getConstantObjectBeanDefinition(createInjectProviderParamsServiceProxy((Service) servImpl, serviceLocator), true);
}
} else {
beanDefinition = getConstantObjectBeanDefinition(servImpl);
beanDefinition = getConstantObjectBeanDefinition(servImpl, true);
}
} else {
throw new IllegalStateException("A custom service must define a service class or instance");
Expand All @@ -115,7 +119,14 @@ protected BeanDefinition getCustomServiceBeanDefinition(CustomService customServ
}

protected static BeanDefinition getConstantObjectBeanDefinition(Object impl) {
return getBeanDefinitionBuilder(ConstantFactoryBean.class).addConstructorArgValue(impl).getBeanDefinition();
return getConstantObjectBeanDefinition(impl, false);
}

protected static BeanDefinition getConstantObjectBeanDefinition(Object impl, boolean inject) {
return getBeanDefinitionBuilder(ConstantFactoryBean.class)
.addConstructorArgValue(impl)
.addConstructorArgValue(inject)
.getBeanDefinition();
}

protected static BeanDefinitionBuilder getBeanDefinitionBuilder(Class<?> beanType) {
Expand Down
6 changes: 6 additions & 0 deletions .../src/main/java/org/mule/runtime/config/internal/context/lazy/LazyMuleArtifactContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.mule.runtime.api.ioc.ConfigurableObjectProvider;
import org.mule.runtime.api.lifecycle.Initialisable;
import org.mule.runtime.api.lifecycle.InitialisationException;
import org.mule.runtime.api.lifecycle.LifecycleException;
import org.mule.runtime.api.lifecycle.Startable;
import org.mule.runtime.api.lock.LockFactory;
import org.mule.runtime.api.memory.management.MemoryManagementService;
Expand Down Expand Up @@ -362,6 +363,11 @@ private void initializeComponents(List<Object> components) {
if (object instanceof MessageProcessorChain) {
// When created it will be initialized
} else {
try {
object = getMuleContext().getInjector().inject(object);
} catch (MuleException e) {
throw new LifecycleException(e, object);
}
getMuleRegistry().applyLifecycle(object, Initialisable.PHASE_NAME);
}
} catch (MuleException e) {
Expand Down
26 changes: 16 additions & 10 deletions ...g/mule/runtime/config/internal/context/lazy/LazySpringMuleContextServiceConfigurator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,38 +102,43 @@ protected void createArtifactServices() {
super.createArtifactServices();

registerBeanDefinition(OBJECT_CONNECTIVITY_TESTER_FACTORY, getBeanDefinition(NoOpConnectivityTesterFactory.class));
registerConstantBeanDefinition(MULE_MEMORY_MANAGEMENT_SERVICE, getMemoryManagementService());
registerConstantBeanDefinition(MULE_MEMORY_MANAGEMENT_SERVICE, getMemoryManagementService(), true);

if (isAddToolingObjectsToRegistry()) {
registerConstantBeanDefinition(CONNECTIVITY_TESTING_SERVICE_KEY,
new LazyConnectivityTestingService(lazyComponentInitializer, () -> getRegistry()
.<ConnectivityTestingService>lookupObject(NON_LAZY_CONNECTIVITY_TESTING_SERVICE)));
.<ConnectivityTestingService>lookupObject(NON_LAZY_CONNECTIVITY_TESTING_SERVICE)),
true);
registerBeanDefinition(NON_LAZY_CONNECTIVITY_TESTING_SERVICE, getBeanDefinition(DefaultConnectivityTestingService.class));

registerConstantBeanDefinition(METADATA_SERVICE_KEY,
new LazyMetadataService(lazyComponentInitializer, () -> getRegistry()
.<MetadataService>lookupObject(NON_LAZY_METADATA_SERVICE_KEY)));
.<MetadataService>lookupObject(NON_LAZY_METADATA_SERVICE_KEY)),
true);
registerBeanDefinition(NON_LAZY_METADATA_SERVICE_KEY, getBeanDefinition(MuleMetadataService.class));

registerConstantBeanDefinition(VALUE_PROVIDER_SERVICE_KEY,
new LazyValueProviderService(lazyComponentInitializer, () -> getRegistry()
.<ValueProviderService>lookupObject(NON_LAZY_VALUE_PROVIDER_SERVICE),
() -> getMuleContext().getConfigurationComponentLocator()));
() -> getMuleContext().getConfigurationComponentLocator()),
true);
registerBeanDefinition(NON_LAZY_VALUE_PROVIDER_SERVICE, getBeanDefinition(MuleValueProviderService.class));

registerConstantBeanDefinition(SAMPLE_DATA_SERVICE_KEY,
new LazySampleDataService(lazyComponentInitializer, () -> getRegistry()
.<SampleDataService>lookupObject(NON_LAZY_SAMPLE_DATA_SERVICE)));
.<SampleDataService>lookupObject(NON_LAZY_SAMPLE_DATA_SERVICE)),
true);
registerBeanDefinition(NON_LAZY_SAMPLE_DATA_SERVICE, getBeanDefinition(MuleSampleDataService.class));

registerBeanDefinition(DEFAULT_METADATA_CACHE_ID_GENERATOR_KEY,
getBeanDefinition(ModelBasedMetadataCacheIdGeneratorFactory.class));
registerConstantBeanDefinition(METADATA_CACHE_ID_GENERATOR_KEY,
new DelegateMetadataCacheIdGeneratorFactory(() -> getRegistry()
.<ModelBasedMetadataCacheIdGeneratorFactory>lookupObject(DEFAULT_METADATA_CACHE_ID_GENERATOR_KEY)));
.<ModelBasedMetadataCacheIdGeneratorFactory>lookupObject(DEFAULT_METADATA_CACHE_ID_GENERATOR_KEY)),
true);
}

registerConstantBeanDefinition(LAZY_COMPONENT_INITIALIZER_SERVICE_KEY, lazyComponentInitializer);
registerConstantBeanDefinition(LAZY_COMPONENT_INITIALIZER_SERVICE_KEY, lazyComponentInitializer, true);

String sharedPartitionedPersistentObjectStorePath =
getArtifactProperties().get(SHARED_PARTITIONED_PERSISTENT_OBJECT_STORE_PATH);
Expand All @@ -142,11 +147,12 @@ protected void createArtifactServices() {
registerBeanDefinition(SHARED_PERSISTENT_OBJECT_STORE_KEY, getBeanDefinition(SharedPartitionedPersistentObjectStore.class));
registerConstantBeanDefinition(SHARED_PERSISTENT_OBJECT_STORE_KEY,
new SharedPartitionedPersistentObjectStore<>(new File(sharedPartitionedPersistentObjectStorePath),
runtimeLockFactory));
runtimeLockFactory),
true);
MuleObjectStoreManager osm = new MuleObjectStoreManager();
osm.setBasePersistentStoreKey(SHARED_PERSISTENT_OBJECT_STORE_KEY);
osm.setBaseTransientStoreKey(BASE_IN_MEMORY_OBJECT_STORE_KEY);
registerConstantBeanDefinition(LAZY_MULE_OBJECT_STORE_MANAGER, osm);
registerConstantBeanDefinition(LAZY_MULE_OBJECT_STORE_MANAGER, osm, true);

if (isAddToolingObjectsToRegistry()) {
registerBeanDefinition(DEFAULT_METADATA_CACHE_MANAGER_KEY,
Expand All @@ -160,7 +166,7 @@ protected void createArtifactServices() {
defaultPersistentMetadataCacheManager.setObjectStoreManager(getRegistry()
.<ObjectStoreManager>lookupObject(LAZY_MULE_OBJECT_STORE_MANAGER));
return defaultPersistentMetadataCacheManager;
}));
}), true);
}
}
}
Expand Down
Loading