diff --git a/src/androidTest/java/com/mixpanel/android/mpmetrics/ComprehensiveSSLSocketFactoryTest.java b/src/androidTest/java/com/mixpanel/android/mpmetrics/ComprehensiveSSLSocketFactoryTest.java new file mode 100644 index 00000000..4bae62f9 --- /dev/null +++ b/src/androidTest/java/com/mixpanel/android/mpmetrics/ComprehensiveSSLSocketFactoryTest.java @@ -0,0 +1,165 @@ +package com.mixpanel.android.mpmetrics; + +import android.content.Context; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.platform.app.InstrumentationRegistry; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import static org.junit.Assert.*; +import java.util.UUID; +import javax.net.ssl.SSLSocketFactory; + +/** + * Comprehensive test that verifies the MPConfig caching fix for GitHub issue #855. + * This test ensures that users can now successfully set SSLSocketFactory on MPConfig instances + * and have those settings apply to MixpanelAPI instances. + */ +@RunWith(AndroidJUnit4.class) +public class ComprehensiveSSLSocketFactoryTest { + + private Context mContext; + + @Before + public void setUp() { + mContext = InstrumentationRegistry.getInstrumentation().getContext(); + // Start each test with a clean cache to avoid test interference + MPConfig.clearInstanceCache(); + } + + /** + * Test the exact scenario described in GitHub issue #855. + * This is the primary use case that was broken before the fix. + */ + @Test + public void testGitHubIssue855_SSLSocketFactoryAssignment() { + String instanceName = "ssl-test-instance"; + String token = UUID.randomUUID().toString(); + + // Step 1: User tries to configure SSL socket factory + // Before fix: This would return a different instance than what MixpanelAPI uses + MPConfig config = MPConfig.getInstance(mContext, instanceName); + SSLSocketFactory originalFactory = config.getSSLSocketFactory(); + assertNotNull("SSL socket factory should be initialized", originalFactory); + + // User sets a custom SSL socket factory (we'll use the original for this test) + config.setSSLSocketFactory(originalFactory); + + // Step 2: User creates MixpanelAPI with the same instance name + // Before fix: This would create a new MPConfig internally, ignoring user's SSL setting + MixpanelAPI mixpanel = MixpanelAPI.getInstance(mContext, token, false, instanceName, false); + + // Step 3: Verify the fix works + MPConfig internalConfig = mixpanel.getMPConfig(); + + // THE FIX: These should be the same instance + assertSame("User config and MixpanelAPI config should be the same instance", + config, internalConfig); + + // SSL socket factory should be preserved + assertSame("SSL socket factory should be preserved", + originalFactory, internalConfig.getSSLSocketFactory()); + } + + /** + * Test that the caching works correctly for default instances (null instance name). + */ + @Test + public void testDefaultInstanceCaching() { + String token = UUID.randomUUID().toString(); + + // Get default config and modify it + MPConfig defaultConfig = MPConfig.getInstance(mContext, null); + SSLSocketFactory customFactory = defaultConfig.getSSLSocketFactory(); + defaultConfig.setSSLSocketFactory(customFactory); + + // Create MixpanelAPI with default instance (no instance name) + MixpanelAPI mixpanel = MixpanelAPI.getInstance(mContext, token, false); + + // Should use the same config instance + assertSame("Default instance should be cached correctly", + defaultConfig, mixpanel.getMPConfig()); + } + + /** + * Test that different instance names get different config instances. + */ + @Test + public void testInstanceIsolation() { + MPConfig config1 = MPConfig.getInstance(mContext, "instance1"); + MPConfig config2 = MPConfig.getInstance(mContext, "instance2"); + MPConfig defaultConfig = MPConfig.getInstance(mContext, null); + + // All should be different instances + assertNotSame("Different instance names should have different configs", config1, config2); + assertNotSame("Named instance should differ from default", config1, defaultConfig); + assertNotSame("Named instance should differ from default", config2, defaultConfig); + + // But same names should return same instances + MPConfig config1Again = MPConfig.getInstance(mContext, "instance1"); + assertSame("Same instance name should return same config", config1, config1Again); + } + + /** + * Test thread safety of the caching mechanism. + */ + @Test + public void testThreadSafety() throws InterruptedException { + final String instanceName = "thread-test"; + final MPConfig[] configs = new MPConfig[10]; + final Thread[] threads = new Thread[10]; + + // Create multiple threads that try to get the same config instance + for (int i = 0; i < threads.length; i++) { + final int index = i; + threads[i] = new Thread(new Runnable() { + @Override + public void run() { + configs[index] = MPConfig.getInstance(mContext, instanceName); + } + }); + } + + // Start all threads + for (Thread thread : threads) { + thread.start(); + } + + // Wait for all threads to complete + for (Thread thread : threads) { + thread.join(); + } + + // All threads should have gotten the same instance + for (int i = 1; i < configs.length; i++) { + assertSame("All threads should get the same config instance", configs[0], configs[i]); + } + } + + /** + * Test that the cache survives multiple MixpanelAPI creations. + */ + @Test + public void testCachePersistenceAcrossMultipleMixpanelInstances() { + String instanceName = "persistence-test"; + String token1 = UUID.randomUUID().toString(); + String token2 = UUID.randomUUID().toString(); + + // Get config and modify it + MPConfig config = MPConfig.getInstance(mContext, instanceName); + SSLSocketFactory factory = config.getSSLSocketFactory(); + config.setSSLSocketFactory(factory); + + // Create first MixpanelAPI instance + MixpanelAPI mixpanel1 = MixpanelAPI.getInstance(mContext, token1, false, instanceName, false); + assertSame("First MixpanelAPI should use cached config", config, mixpanel1.getMPConfig()); + + // Create second MixpanelAPI instance with different token but same instance name + MixpanelAPI mixpanel2 = MixpanelAPI.getInstance(mContext, token2, false, instanceName, false); + assertSame("Second MixpanelAPI should use same cached config", config, mixpanel2.getMPConfig()); + + // Both MixpanelAPI instances should use the same config + assertSame("Both MixpanelAPI instances should share the same config", + mixpanel1.getMPConfig(), mixpanel2.getMPConfig()); + } +} \ No newline at end of file diff --git a/src/androidTest/java/com/mixpanel/android/mpmetrics/IntegrationTestSSLSocketFactoryFix.java b/src/androidTest/java/com/mixpanel/android/mpmetrics/IntegrationTestSSLSocketFactoryFix.java new file mode 100644 index 00000000..d27cff48 --- /dev/null +++ b/src/androidTest/java/com/mixpanel/android/mpmetrics/IntegrationTestSSLSocketFactoryFix.java @@ -0,0 +1,83 @@ +package com.mixpanel.android.mpmetrics; + +import android.content.Context; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.platform.app.InstrumentationRegistry; +import org.junit.Test; +import org.junit.runner.RunWith; +import static org.junit.Assert.*; +import java.util.UUID; +import javax.net.ssl.SSLSocketFactory; + +/** + * Integration test that demonstrates the exact scenario described in the GitHub issue. + * Before the fix: Setting SSLSocketFactory would not work because different instances were returned. + * After the fix: Setting SSLSocketFactory works because the same instance is returned and used. + */ +@RunWith(AndroidJUnit4.class) +public class IntegrationTestSSLSocketFactoryFix { + + @Test + public void testGitHubIssueScenario() { + // Clear cache to start fresh + MPConfig.clearInstanceCache(); + + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + String instanceName = "my-custom-instance"; + String fakeToken = UUID.randomUUID().toString(); + + // Step 1: This is what users were trying to do (the old broken way described in the issue) + // Before our fix, this would create a new MPConfig instance that would not be used by MixpanelAPI + MPConfig userConfig = MPConfig.getInstance(context, instanceName); + + // Get the original SSL socket factory + SSLSocketFactory originalFactory = userConfig.getSSLSocketFactory(); + assertNotNull("Original SSL socket factory should not be null", originalFactory); + + // Simulate setting a custom SSL socket factory + // For this test we just set it to the original factory to verify consistency + userConfig.setSSLSocketFactory(originalFactory); + + // Step 2: User creates MixpanelAPI instance with the same instance name + // Before the fix: This would create a new MPConfig instance, ignoring the user's configuration + // After the fix: This should use the same MPConfig instance that the user configured + MixpanelAPI mixpanel = MixpanelAPI.getInstance(context, fakeToken, false, instanceName, false); + + // Step 3: Verify the fix - both should reference the same MPConfig instance + MPConfig internalConfig = mixpanel.getMPConfig(); + + // THE FIX: These should now be the same instance + assertSame("MPConfig.getInstance() should return the same instance used by MixpanelAPI", + userConfig, internalConfig); + + // This means the SSL socket factory setting is preserved + assertSame("SSL socket factory should be preserved across getInstance calls", + userConfig.getSSLSocketFactory(), internalConfig.getSSLSocketFactory()); + + // Step 4: Verify that getting the config again still returns the same instance + MPConfig userConfig2 = MPConfig.getInstance(context, instanceName); + assertSame("Subsequent calls to MPConfig.getInstance() should return the same cached instance", + userConfig, userConfig2); + } + + @Test + public void testDefaultInstanceIsConsistent() { + // Clear cache to start fresh + MPConfig.clearInstanceCache(); + + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + String fakeToken = UUID.randomUUID().toString(); + + // Test with default instance (null instanceName) + MPConfig userConfig = MPConfig.getInstance(context, null); + SSLSocketFactory customFactory = userConfig.getSSLSocketFactory(); + userConfig.setSSLSocketFactory(customFactory); + + // Create MixpanelAPI with default instance (no instanceName specified) + MixpanelAPI mixpanel = MixpanelAPI.getInstance(context, fakeToken, false); + MPConfig internalConfig = mixpanel.getMPConfig(); + + // Should be the same instance for default case too + assertSame("Default MPConfig instance should be consistent", userConfig, internalConfig); + } +} \ No newline at end of file diff --git a/src/androidTest/java/com/mixpanel/android/mpmetrics/MPConfigTest.java b/src/androidTest/java/com/mixpanel/android/mpmetrics/MPConfigTest.java index 1852a8ad..a94fefe1 100644 --- a/src/androidTest/java/com/mixpanel/android/mpmetrics/MPConfigTest.java +++ b/src/androidTest/java/com/mixpanel/android/mpmetrics/MPConfigTest.java @@ -1,5 +1,6 @@ package com.mixpanel.android.mpmetrics; +import android.content.Context; import android.os.Bundle; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -221,6 +222,60 @@ public void testShouldGzipRequestPayload() { } + @Test + public void testMPConfigInstanceCaching() { + // Clear cache first to ensure clean state + MPConfig.clearInstanceCache(); + + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + + // Test that same instance is returned for same parameters + MPConfig config1 = MPConfig.getInstance(context, null); + MPConfig config2 = MPConfig.getInstance(context, null); + assertTrue("Same MPConfig instance should be returned for same context and instanceName", config1 == config2); + + // Test with named instances + MPConfig namedConfig1 = MPConfig.getInstance(context, "test-instance"); + MPConfig namedConfig2 = MPConfig.getInstance(context, "test-instance"); + assertTrue("Same MPConfig instance should be returned for same context and instanceName", namedConfig1 == namedConfig2); + + // Test that different instance names return different instances + MPConfig differentConfig = MPConfig.getInstance(context, "different-instance"); + assertFalse("Different MPConfig instances should be returned for different instanceNames", namedConfig1 == differentConfig); + + // Test that null and named instances are different + assertFalse("Default instance should be different from named instance", config1 == namedConfig1); + } + + @Test + public void testSSLSocketFactoryConsistency() { + // Clear cache first to ensure clean state + MPConfig.clearInstanceCache(); + + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + String instanceName = "ssl-test-instance"; + + // Get config and set a custom SSLSocketFactory + MPConfig config1 = MPConfig.getInstance(context, instanceName); + javax.net.ssl.SSLSocketFactory originalFactory = config1.getSSLSocketFactory(); + + // Create a mock factory (we'll just use the original as a placeholder for this test) + javax.net.ssl.SSLSocketFactory customFactory = originalFactory; + config1.setSSLSocketFactory(customFactory); + + // Get the same config instance again and verify the factory is preserved + MPConfig config2 = MPConfig.getInstance(context, instanceName); + assertTrue("Should return the same MPConfig instance", config1 == config2); + assertTrue("Custom SSLSocketFactory should be preserved", config2.getSSLSocketFactory() == customFactory); + + // Test that MixpanelAPI uses the same config instance + String fakeToken = UUID.randomUUID().toString(); + MixpanelAPI mixpanel = MixpanelAPI.getInstance(context, fakeToken, false, instanceName, false); + MPConfig mixpanelConfig = mixpanel.getMPConfig(); + assertTrue("MixpanelAPI should use the same MPConfig instance", mixpanelConfig == config1); + assertTrue("MixpanelAPI should see the custom SSLSocketFactory", mixpanelConfig.getSSLSocketFactory() == customFactory); + } + private MPConfig mpConfig(final Bundle metaData) { return new MPConfig(metaData, InstrumentationRegistry.getInstrumentation().getContext(), null); } diff --git a/src/androidTest/java/com/mixpanel/android/mpmetrics/SSLSocketFactoryTest.java b/src/androidTest/java/com/mixpanel/android/mpmetrics/SSLSocketFactoryTest.java new file mode 100644 index 00000000..49ed12f6 --- /dev/null +++ b/src/androidTest/java/com/mixpanel/android/mpmetrics/SSLSocketFactoryTest.java @@ -0,0 +1,82 @@ +package com.mixpanel.android.mpmetrics; + +import android.content.Context; +import android.os.Bundle; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.platform.app.InstrumentationRegistry; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.junit.Assert.assertTrue; + +import java.util.UUID; + +import javax.net.ssl.SSLSocketFactory; + +/** + * Test class to demonstrate and verify the fix for the SSL socket factory issue. + * This test validates that when a user calls MPConfig.getInstance() to set an SSLSocketFactory, + * the same instance is used by MixpanelAPI internally. + */ +@RunWith(AndroidJUnit4.class) +public class SSLSocketFactoryTest { + + @Test + public void testSSLSocketFactoryConsistencyWithMixpanelAPI() { + // Clear cache first to ensure clean state + MPConfig.clearInstanceCache(); + + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + String instanceName = "ssl-consistency-test"; + String fakeToken = UUID.randomUUID().toString(); + + // Step 1: User calls MPConfig.getInstance() to configure SSL + MPConfig userConfig = MPConfig.getInstance(context, instanceName); + SSLSocketFactory originalFactory = userConfig.getSSLSocketFactory(); + + // Simulate setting a custom SSL socket factory (we'll use the original for this test) + userConfig.setSSLSocketFactory(originalFactory); + + // Step 2: User creates MixpanelAPI instance with the same instanceName + MixpanelAPI mixpanel = MixpanelAPI.getInstance(context, fakeToken, false, instanceName, false); + + // Step 3: Verify that MixpanelAPI uses the same MPConfig instance + MPConfig mixpanelConfig = mixpanel.getMPConfig(); + + // This should now pass with our fix - same instance should be returned + assertTrue("MPConfig.getInstance() should return the same instance that MixpanelAPI uses", + userConfig == mixpanelConfig); + + // This should also pass - the SSL socket factory should be the same + assertTrue("SSLSocketFactory should be consistent between user config and MixpanelAPI config", + userConfig.getSSLSocketFactory() == mixpanelConfig.getSSLSocketFactory()); + } + + @Test + public void testDifferentInstanceNamesGetDifferentConfigs() { + // Clear cache first to ensure clean state + MPConfig.clearInstanceCache(); + + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + + // Get configs for different instance names + MPConfig config1 = MPConfig.getInstance(context, "instance1"); + MPConfig config2 = MPConfig.getInstance(context, "instance2"); + MPConfig config3 = MPConfig.getInstance(context, null); // default instance + + // All should be different instances + assertTrue("Different instance names should return different MPConfig instances", + config1 != config2); + assertTrue("Named instance should be different from default instance", + config1 != config3); + assertTrue("Different named instances should be different from each other", + config2 != config3); + + // But getting the same instance name multiple times should return the same instance + MPConfig config1Again = MPConfig.getInstance(context, "instance1"); + assertTrue("Same instance name should return the same MPConfig instance", + config1 == config1Again); + } +} \ No newline at end of file diff --git a/src/main/java/com/mixpanel/android/mpmetrics/MPConfig.java b/src/main/java/com/mixpanel/android/mpmetrics/MPConfig.java index 315cf5f8..787d2e4d 100644 --- a/src/main/java/com/mixpanel/android/mpmetrics/MPConfig.java +++ b/src/main/java/com/mixpanel/android/mpmetrics/MPConfig.java @@ -16,6 +16,7 @@ import java.security.GeneralSecurityException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLSocketFactory; @@ -103,27 +104,39 @@ public class MPConfig { // Name for persistent storage of app referral SharedPreferences /* package */ static final String REFERRER_PREFS_NAME = "com.mixpanel.android.mpmetrics.ReferralInfo"; + // Cache for MPConfig instances to ensure the same instance is returned for the same context + instanceName combination + private static final Map sInstanceCache = new ConcurrentHashMap<>(); + + // Lock object for synchronizing cache operations + private static final Object sInstanceCacheLock = new Object(); + /** - * Retrieves a new instance of MPConfig with configuration settings loaded from the provided context. - * This method creates a new instance each time it is called, allowing for multiple configurations - * within the same application. + * Retrieves an instance of MPConfig with configuration settings loaded from the provided context. + * Since version 7.4.0, this method now caches instances based on the context package name and + * instance name combination, ensuring that the same MPConfig instance is returned for identical + * parameters. This change addresses issues where setting properties like SSLSocketFactory on + * retrieved instances would not affect the instances used internally by MixpanelAPI. * - * Since version 7.4.0, MPConfig is no longer a Singleton, in favor of supporting multiple, - * distinct configurations for different Mixpanel instances. This change allows greater flexibility - * in scenarios where different parts of an application require different Mixpanel configurations, - * such as different endpoints or settings. + * The caching behavior allows for multiple configurations within the same application while + * ensuring consistency between instances retrieved by user code and those used internally. + * Each unique combination of application context and instance name will result in a single, + * shared MPConfig instance. * - * It's important for users of this method to manage the lifecycle of the returned MPConfig instances - * themselves. Each call will result in a new configuration instance based on the application's - * metadata, and it's the responsibility of the caller to maintain any necessary references to these - * instances to use them later in their application. + * It's important for users of this method to understand that the returned MPConfig instances + * are now cached and shared. Changes made to configuration properties will affect all code + * that retrieves the same instance. * * @param context The context used to load Mixpanel configuration. It's recommended to provide * an ApplicationContext to avoid potential memory leaks. - * @return A new instance of MPConfig with settings loaded from the context's application metadata. + * @param instanceName The instance name for this configuration. Use null for default instance. + * @return A cached instance of MPConfig with settings loaded from the context's application metadata. */ public static MPConfig getInstance(Context context, @Nullable String instanceName) { - return readConfig(context.getApplicationContext(), instanceName); + final Context appContext = context.getApplicationContext(); + final String packageName = appContext.getPackageName(); + final String cacheKey = packageName + ":" + (instanceName != null ? instanceName : "default"); + + return sInstanceCache.computeIfAbsent(cacheKey, key -> readConfig(appContext, instanceName)); } /** @@ -472,6 +485,13 @@ public void setProxyServerInteractor(ProxyServerInteractor interactor) { } } + // Package access for testing only- clears the instance cache + /* package */ static void clearInstanceCache() { + synchronized (sInstanceCacheLock) { + sInstanceCache.clear(); + } + } + @Override public String toString() { return "Mixpanel (" + VERSION + ") configured with:\n" +