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.

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

1.x: add shorter RxJavaPlugin class lookup approach. #3513

Merged
merged 1 commit into from
Nov 12, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
66 changes: 56 additions & 10 deletions src/main/java/rx/plugins/RxJavaPlugins.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package rx.plugins;

import java.util.*;
import java.util.concurrent.atomic.AtomicReference;

/**
Expand All @@ -26,7 +27,22 @@
* property names)</li>
* <li>default implementation</li>
* </ol>
*
* <p>In addition to the {@code rxjava.plugin.[simple classname].implementation} system properties,
* you can define two system property:<br>
* <pre><code>
* rxjava.plugin.[index].class}
* rxjava.plugin.[index].impl}
* </code></pre>
*
* Where the {@code .class} property contains the simple classname from above and the {@code .impl}
* contains the fully qualified name of the implementation class. The {@code [index]} can be
* any short string or number of your chosing. For example, you can now define a custom
* {@code RxJavaErrorHandler} via two system property:
* <pre><code>
* rxjava.plugin.1.class=RxJavaErrorHandler
* rxjava.plugin.1.impl=some.package.MyRxJavaErrorHandler
* </code></pre>
*
* @see <a href="https://github.com/ReactiveX/RxJava/wiki/Plugins">RxJava Wiki: Plugins</a>
*/
public class RxJavaPlugins {
Expand Down Expand Up @@ -64,13 +80,12 @@ public static RxJavaPlugins getInstance() {
* <p>
* Override the default by calling {@link #registerErrorHandler(RxJavaErrorHandler)} or by setting the
* property {@code rxjava.plugin.RxJavaErrorHandler.implementation} with the full classname to load.
*
* @return {@link RxJavaErrorHandler} implementation to use
*/
public RxJavaErrorHandler getErrorHandler() {
if (errorHandler.get() == null) {
// check for an implementation from System.getProperty first
Object impl = getPluginImplementationViaProperty(RxJavaErrorHandler.class);
Object impl = getPluginImplementationViaProperty(RxJavaErrorHandler.class, System.getProperties());
if (impl == null) {
// nothing set via properties so initialize with default
errorHandler.compareAndSet(null, DEFAULT_ERROR_HANDLER);
Expand Down Expand Up @@ -112,7 +127,7 @@ public void registerErrorHandler(RxJavaErrorHandler impl) {
public RxJavaObservableExecutionHook getObservableExecutionHook() {
if (observableExecutionHook.get() == null) {
// check for an implementation from System.getProperty first
Object impl = getPluginImplementationViaProperty(RxJavaObservableExecutionHook.class);
Object impl = getPluginImplementationViaProperty(RxJavaObservableExecutionHook.class, System.getProperties());
if (impl == null) {
// nothing set via properties so initialize with default
observableExecutionHook.compareAndSet(null, RxJavaObservableExecutionHookDefault.getInstance());
Expand Down Expand Up @@ -141,15 +156,46 @@ public void registerObservableExecutionHook(RxJavaObservableExecutionHook impl)
}
}

private static Object getPluginImplementationViaProperty(Class<?> pluginClass) {
String classSimpleName = pluginClass.getSimpleName();
/* test */ static Object getPluginImplementationViaProperty(Class<?> pluginClass, Properties props) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change test to VisibleForTesting? It's pretty common pattern.

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find this VisibleForTesting in RxJava.

Copy link
Member

Choose a reason for hiding this comment

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

VisibleForTesting is in Guava

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant not annotation but comment.

On Wed, Nov 11, 2015, 21:32 Shixiong Zhu [email protected] wrote:

In src/main/java/rx/plugins/RxJavaPlugins.java
#3513 (comment):

@@ -141,7 +156,7 @@ public void registerObservableExecutionHook(RxJavaObservableExecutionHook impl)
}
}

  • private static Object getPluginImplementationViaProperty(Class<?> pluginClass) {
  • /* test */ static Object getPluginImplementationViaProperty(Class<?> pluginClass, Properties props) {

VisibleForTesting is in Guava


Reply to this email directly or view it on GitHub
https://github.com/ReactiveX/RxJava/pull/3513/files#r44567271.

@artem_zin

final String classSimpleName = pluginClass.getSimpleName();
/*
* Check system properties for plugin class.
* <p>
* This will only happen during system startup thus it's okay to use the synchronized
* System.getProperties as it will never get called in normal operations.
*/
String implementingClass = System.getProperty("rxjava.plugin." + classSimpleName + ".implementation");

final String pluginPrefix = "rxjava.plugin.";

String defaultKey = pluginPrefix + classSimpleName + ".implementation";
String implementingClass = props.getProperty(defaultKey);

if (implementingClass == null) {
final String classSuffix = ".class";
final String implSuffix = ".impl";

for (Map.Entry<Object, Object> e : props.entrySet()) {
String key = e.getKey().toString();
if (key.startsWith(pluginPrefix) && key.endsWith(classSuffix)) {
String value = e.getValue().toString();

if (classSimpleName.equals(value)) {
String index = key.substring(0, key.length() - classSuffix.length()).substring(pluginPrefix.length());

String implKey = pluginPrefix + index + implSuffix;

implementingClass = props.getProperty(implKey);

if (implementingClass == null) {
throw new RuntimeException("Implementing class declaration for " + classSimpleName + " missing: " + implKey);
}

break;
}
}
}
}

if (implementingClass != null) {
try {
Class<?> cls = Class.forName(implementingClass);
Expand All @@ -165,9 +211,9 @@ private static Object getPluginImplementationViaProperty(Class<?> pluginClass) {
} catch (IllegalAccessException e) {
throw new RuntimeException(classSimpleName + " implementation not able to be accessed: " + implementingClass, e);
}
} else {
return null;
}

return null;
}

/**
Expand All @@ -183,7 +229,7 @@ private static Object getPluginImplementationViaProperty(Class<?> pluginClass) {
public RxJavaSchedulersHook getSchedulersHook() {
if (schedulersHook.get() == null) {
// check for an implementation from System.getProperty first
Object impl = getPluginImplementationViaProperty(RxJavaSchedulersHook.class);
Object impl = getPluginImplementationViaProperty(RxJavaSchedulersHook.class, System.getProperties());
if (impl == null) {
// nothing set via properties so initialize with default
schedulersHook.compareAndSet(null, RxJavaSchedulersHook.getDefaultInstance());
Expand Down
46 changes: 33 additions & 13 deletions src/test/java/rx/plugins/RxJavaPluginsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,12 @@
*/
package rx.plugins;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.Calendar;
import java.util.Collections;
import java.util.Date;
import static org.junit.Assert.*;

import java.util.*;
import java.util.concurrent.TimeUnit;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.*;

import rx.Observable;
import rx.Subscriber;
Expand Down Expand Up @@ -251,4 +242,33 @@ private static String getFullClassNameForTestClass(Class<?> cls) {
return RxJavaPlugins.class.getPackage()
.getName() + "." + RxJavaPluginsTest.class.getSimpleName() + "$" + cls.getSimpleName();
}

@Test
public void testShortPluginDiscovery() {
Properties props = new Properties();

props.setProperty("rxjava.plugin.1.class", "Map");
props.setProperty("rxjava.plugin.1.impl", "java.util.HashMap");

props.setProperty("rxjava.plugin.xyz.class", "List");
props.setProperty("rxjava.plugin.xyz.impl", "java.util.ArrayList");


Object o = RxJavaPlugins.getPluginImplementationViaProperty(Map.class, props);

assertTrue("" + o, o instanceof HashMap);

o = RxJavaPlugins.getPluginImplementationViaProperty(List.class, props);

assertTrue("" + o, o instanceof ArrayList);
}

@Test(expected = RuntimeException.class)
public void testShortPluginDiscoveryMissing() {
Properties props = new Properties();

props.setProperty("rxjava.plugin.1.class", "Map");

RxJavaPlugins.getPluginImplementationViaProperty(Map.class, props);
}
}