Skip to content

Commit

Permalink
W-17463948: Prevent double injection of objects in registry (#14066)
Browse files Browse the repository at this point in the history
  • Loading branch information
elrodro83 authored Dec 18, 2024
1 parent d85f2f8 commit 43576d1
Show file tree
Hide file tree
Showing 13 changed files with 97 additions and 73 deletions.
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
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
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
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
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
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
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
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
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
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

0 comments on commit 43576d1

Please sign in to comment.