Fixes dependency on concrete impl and code smells (#312)

* Fixes dependency on concrete impl and code smells

* Moved Switcher operations from Builder to interface

* Removed get optional state from Switcher minimal contract

Author: petruki

src/main/java/com/github/switcherapi/client/SwitcherContextBase.java

@@ -310,7 +310,8 @@ public static void watchSnapshot(SnapshotEventHandler handler) {
 		}
 
 		if (watcher == null) {
-			watcher = new SnapshotWatcher((SwitcherLocalService) instance, handler);
+			watcher = new SnapshotWatcher((SwitcherLocalService) instance, handler,
+					contextStr(ContextKey.SNAPSHOT_LOCATION));
 		}
 
 		initWatcherExecutorService();

src/main/java/com/github/switcherapi/client/model/AsyncSwitcher.java

@@ -24,13 +24,16 @@ public class AsyncSwitcher implements Runnable {
 
 	private final ExecutorService executorService;
 
-	private final Switcher switcher;
+	private final SwitcherInterface switcherInterface;
+
+	private final long delay;
 
 	private long nextRun = 0;
 
-	public AsyncSwitcher(final Switcher switcher) {
+	public AsyncSwitcher(final SwitcherInterface switcherInterface, long delay) {
 		this.executorService = Executors.newCachedThreadPool();
-		this.switcher = switcher;
+		this.switcherInterface = switcherInterface;
+		this.delay = delay;
 	}
 
 	/**
@@ -43,19 +46,21 @@ public synchronized void execute() {
 		if (nextRun < System.currentTimeMillis()) {
 			SwitcherUtils.debug(logger, "Running AsyncSwitcher");
 
-			this.nextRun = System.currentTimeMillis() + switcher.delay;
+			this.nextRun = System.currentTimeMillis() + this.delay;
 			this.executorService.submit(this);
 		}
 	}
 
 	@Override
 	public void run() {
 		try {
-			final CriteriaResponse response = switcher.getContext().executeCriteria(switcher);
-			switcher.getHistoryExecution().removeIf(item ->
-					switcher.getSwitcherKey().equals(item.getSwitcherKey()) &&
-					switcher.getEntry().equals(item.getEntry()));
-			switcher.getHistoryExecution().add(response);
+			final CriteriaResponse response = switcherInterface.executeCriteria();
+
+			switcherInterface.getHistoryExecution().removeIf(item ->
+					switcherInterface.getSwitcherKey().equals(item.getSwitcherKey()) &&
+							switcherInterface.getEntry().equals(item.getEntry()));
+
+			switcherInterface.getHistoryExecution().add(response);
 		} catch (SwitcherException e) {
 			logger.error(e);
 		}

src/main/java/com/github/switcherapi/client/model/Switcher.java

@@ -42,19 +42,6 @@ public Switcher(final String switcherKey, final SwitcherExecutor context) {
 		this.switcherKey = switcherKey;
 		this.historyExecution = new HashSet<>();
 	}
-	
-	private boolean canUseAsync() {
-		return super.delay > 0 && !this.historyExecution.isEmpty();
-	}
-	
-	private Optional<CriteriaResponse> getFromHistory() {
-		for (CriteriaResponse criteriaResponse : historyExecution) {
-			if (criteriaResponse.getEntry().equals(getEntry())) {
-				return Optional.of(criteriaResponse);
-			}
-		}
-		return Optional.empty();
-	}
 
 	@Override
 	public Switcher build() {
@@ -103,7 +90,7 @@ public CriteriaResponse submit() throws SwitcherException {
 
 		if (canUseAsync()) {
 			if (asyncSwitcher == null) {
-				asyncSwitcher = new AsyncSwitcher(this);
+				asyncSwitcher = new AsyncSwitcher(this, super.delay);
 			}
 
 			asyncSwitcher.execute();
@@ -117,6 +104,26 @@ public CriteriaResponse submit() throws SwitcherException {
 		this.historyExecution.add(response);
 		return response;
 	}
+
+	@Override
+	public CriteriaResponse executeCriteria() {
+		return this.context.executeCriteria(this);
+	}
+
+	@Override
+	public synchronized Set<CriteriaResponse> getHistoryExecution() {
+		return this.historyExecution;
+	}
+
+	@Override
+	public String getSwitcherKey() {
+		return this.switcherKey;
+	}
+
+	@Override
+	public List<Entry> getEntry() {
+		return this.entry;
+	}
 
 	/**
 	 * This method builds up the request made by the client to reach the Switcher API.
@@ -129,24 +136,29 @@ public GsonInputRequest getInputRequest() {
 						this.entry.toArray(new Entry[0]) : null);
 	}
 
-	public boolean isBypassMetrics() {
-		return bypassMetrics;
-	}
-
-	public String getSwitcherKey() {
-		return this.switcherKey;
+	public long getDelay() {
+		return super.delay;
 	}
 
-	public List<Entry> getEntry() {
-		return this.entry;
+	public boolean isBypassMetrics() {
+		return bypassMetrics;
 	}
 
 	public void resetEntry() {
 		this.entry = new ArrayList<>();
 	}
 
-	public synchronized Set<CriteriaResponse> getHistoryExecution() {
-		return this.historyExecution;
+	private boolean canUseAsync() {
+		return super.delay > 0 && !this.historyExecution.isEmpty();
+	}
+
+	private Optional<CriteriaResponse> getFromHistory() {
+		for (CriteriaResponse criteriaResponse : historyExecution) {
+			if (criteriaResponse.getEntry().equals(getEntry())) {
+				return Optional.of(criteriaResponse);
+			}
+		}
+		return Optional.empty();
 	}
 
 	@Override

src/main/java/com/github/switcherapi/client/model/SwitcherBuilder.java

@@ -1,10 +1,7 @@
 package com.github.switcherapi.client.model;
 
-import com.github.switcherapi.client.SwitcherContext;
 import com.github.switcherapi.client.SwitcherExecutor;
 import com.github.switcherapi.client.exception.SwitcherContextException;
-import com.github.switcherapi.client.exception.SwitcherException;
-import com.github.switcherapi.client.model.response.CriteriaResponse;
 import org.apache.commons.lang3.StringUtils;
 
 import java.util.ArrayList;
@@ -16,7 +13,7 @@
  * 
  * @author Roger Floriano (petruki)
  */
-public abstract class SwitcherBuilder {
+public abstract class SwitcherBuilder implements SwitcherInterface {
 
 	protected final SwitcherExecutor context;
 
@@ -170,68 +167,6 @@ public SwitcherBuilder bypassMetrics() {
 		return this;
 	}
 
-	/**
-	 * This method builds the Switcher object.<br>
-	 * Uses to isolate Switcher creation from the execution.<br>
-	 *
-	 * For example:
-	 * <pre>
-	 * Switcher switcher = SwitcherContext
-	 * 	.getSwitcher(MY_SWITCHER)
-	 * 	.remote(true)
-	 * 	.throttle(1000)
-	 * 	.checkValue("value")
-	 * 	.build();
-	 * </pre>
-	 *
-	 * @return {@link Switcher}
-	 */
-	public abstract Switcher build();
-	
-	/**
-	 * Prepare the Switcher including a list of inputs necessary to run the criteria afterward.
-	 * 
-	 * @param entry input object
-	 * @return {@link Switcher}
-	 */
-	public abstract Switcher prepareEntry(final List<Entry> entry);
-	
-	/**
-	 * Prepare the Switcher including a list of inputs necessary to run the criteria afterward.
-	 * 
-	 * @param entry input object
-	 * @param add if false, the list will be cleaned and the entry provided will be the only input for this Switcher.
-	 * @return {@link Switcher}
-	 */
-	public abstract Switcher prepareEntry(final Entry entry, final boolean add);
-	
-	/**
-	 * It adds an input to the list of inputs.
-	 * <br>Under the table it calls {@link #prepareEntry(Entry, boolean)} passing true to the second argument.
-	 * 
-	 * @param entry input object
-	 * @return {@link Switcher}
-	 */
-	public abstract Switcher prepareEntry(final Entry entry);
-	
-	/**
-	 * Execute criteria based on a given switcher key provided via {@link SwitcherContext#getSwitcher(String)}.
-	 * <br>The detailed result is available in list of {@link CriteriaResponse}.
-	 * 
-	 * @return criteria result
-	 * @throws SwitcherException connectivity or criteria errors regarding reading malformed snapshots
-	 */
-	public abstract boolean isItOn() throws SwitcherException;
-
-	/**
-	 * Execute criteria based on a given switcher key provided via {@link SwitcherContext#getSwitcher(String)}.
-	 * <br>The detailed result is available in list of {@link CriteriaResponse}.
-	 *
-	 * @return {@link CriteriaResponse}
-	 * @throws SwitcherException connectivity or criteria errors regarding reading malformed snapshots
-	 */
-	public abstract CriteriaResponse submit() throws SwitcherException;
-
 	public boolean isRemote() {
 		return remote;
 	}

src/main/java/com/github/switcherapi/client/model/SwitcherInterface.java

@@ -0,0 +1,111 @@
+package com.github.switcherapi.client.model;
+
+import com.github.switcherapi.client.SwitcherContext;
+import com.github.switcherapi.client.exception.SwitcherException;
+import com.github.switcherapi.client.model.response.CriteriaResponse;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Defines minimal contract for Switcher implementations for:
+ *
+ * <ul>
+ *     <li>Switcher creation</li>
+ *     <li>Switcher execution</li>
+ *     <li>Switcher get input/output</li>
+ * </ul>
+ */
+public interface SwitcherInterface {
+
+	/**
+	 * This method builds the Switcher object.<br>
+	 * Uses to isolate Switcher creation from the execution.<br>
+	 *
+	 * For example:
+	 * <pre>
+	 * Switcher switcher = SwitcherContext
+	 * 	.getSwitcher(MY_SWITCHER)
+	 * 	.remote(true)
+	 * 	.throttle(1000)
+	 * 	.checkValue("value")
+	 * 	.build();
+	 * </pre>
+	 *
+	 * @return {@link Switcher}
+	 */
+	Switcher build();
+
+	/**
+	 * Prepare the Switcher including a list of inputs necessary to run the criteria afterward.
+	 *
+	 * @param entry input object
+	 * @return {@link Switcher}
+	 */
+	Switcher prepareEntry(final List<Entry> entry);
+
+	/**
+	 * Prepare the Switcher including a list of inputs necessary to run the criteria afterward.
+	 *
+	 * @param entry input object
+	 * @param add if false, the list will be cleaned and the entry provided will be the only input for this Switcher.
+	 * @return {@link Switcher}
+	 */
+	Switcher prepareEntry(final Entry entry, final boolean add);
+
+	/**
+	 * It adds an input to the list of inputs.
+	 * <br>Under the table it calls {@link #prepareEntry(Entry, boolean)} passing true to the second argument.
+	 *
+	 * @param entry input object
+	 * @return {@link Switcher}
+	 */
+	Switcher prepareEntry(final Entry entry);
+
+	/**
+	 * Execute criteria based on a given switcher key provided via {@link SwitcherContext#getSwitcher(String)}.
+	 * <br>The detailed result is available in list of {@link CriteriaResponse}.
+	 *
+	 * @return criteria result
+	 * @throws SwitcherException connectivity or criteria errors regarding reading malformed snapshots
+	 */
+	boolean isItOn() throws SwitcherException;
+
+	/**
+	 * Execute criteria based on a given switcher key provided via {@link SwitcherContext#getSwitcher(String)}.
+	 * <br>The detailed result is available in list of {@link CriteriaResponse}.
+	 *
+	 * @return {@link CriteriaResponse}
+	 * @throws SwitcherException connectivity or criteria errors regarding reading malformed snapshots
+	 */
+	CriteriaResponse submit() throws SwitcherException;
+
+	/**
+	 * Execute the criteria evaluation.
+	 *
+	 * @return the criteria response
+	 */
+	CriteriaResponse executeCriteria();
+
+	/**
+	 * Get the history of executions.
+	 *
+	 * @return the history of executions
+	 */
+	Set<CriteriaResponse> getHistoryExecution();
+
+	/**
+	 * Get the key of the switcher.
+	 *
+	 * @return the key of the switcher
+	 */
+	String getSwitcherKey();
+
+	/**
+	 * Get the entry input list for the switcher.
+	 *
+	 * @return the entry of the switcher
+	 */
+	List<Entry> getEntry();
+
+}

src/main/java/com/github/switcherapi/client/model/response/CriteriaResponse.java

@@ -47,8 +47,8 @@ public CriteriaResponse buildFromSwitcher(Switcher switcher) {
 		this.entry = switcher.getEntry();
 
 		if (Objects.nonNull(entry)) {
-			for (Entry entry : entry) {
-				if (entryWhen.containsKey(entry.getStrategy()) && !entryWhen.get(entry.getStrategy()).contains(entry.getInput())) {
+			for (Entry inputEntry : entry) {
+				if (!isEntryMatching(inputEntry)) {
 					return new CriteriaResponse(!this.result, this.reason, switcher);
 				}
 			}
@@ -57,6 +57,11 @@ public CriteriaResponse buildFromSwitcher(Switcher switcher) {
 		return this;
 	}
 
+	private boolean isEntryMatching(Entry inputEntry) {
+		return entryWhen.containsKey(inputEntry.getStrategy()) &&
+				entryWhen.get(inputEntry.getStrategy()).contains(inputEntry.getInput());
+	}
+
 	public static CriteriaResponse buildFromDefault(Switcher switcher) {
 		return new CriteriaResponse(Boolean.parseBoolean(switcher.getDefaultResult()), DEFAULT_REASON, switcher);
 	}

src/main/java/com/github/switcherapi/client/test/SwitcherTestExtension.java

@@ -34,9 +34,9 @@ public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContex
 		SwitcherTest switcherTest = context.getRequiredTestMethod().getAnnotation(SwitcherTest.class);
 
 		if (switcherTest.abTest()) {
-			final SwitcherTestTemplate templateA = new SwitcherTestTemplate(switcherTest, true);
-			final SwitcherTestTemplate templateB = new SwitcherTestTemplate(switcherTest);
-			return Stream.of(templateA, templateB);
+			return Stream.of(
+					new SwitcherTestTemplate(switcherTest, true),
+					new SwitcherTestTemplate(switcherTest));
 		}
 
 		final SwitcherTestTemplate template = new SwitcherTestTemplate(switcherTest);