Skip to content

Commit

Permalink
Merge pull request #136 from Flipkart/code_optimizations
Browse files Browse the repository at this point in the history 
code optimizations ( Please Review , Don't Merge)
  • Loading branch information
@yasirmhd
yasirmhd authored Mar 6, 2017
2 parents 0a7a618 + 731eeba commit 8ef1113
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 49 deletions.
8 changes: 1 addition & 7 deletions batching-gson/src/test/java/com/flipkart/batching/gson/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,10 @@ public class Utils {
* @param size
* @return dataList
*/
private static Data eventData;

public static ArrayList<Data> fakeCollection(int size) {
ArrayList<Data> dataList = new ArrayList<>();
for (int i = 0; i < size; i++) {
eventData = new EventData();
Data eventData = new EventData();
eventData.setEventId(System.currentTimeMillis() + System.nanoTime() + i);
dataList.add(eventData);
}
Expand All @@ -73,10 +71,6 @@ public CustomTagData(Tag tag, JSONObject event) {
this.event = event;
}

public JSONObject getEvent() {
return event;
}

@Override
public String toString() {
return super.toString() + ":" + getEventId();
Expand Down
3 changes: 1 addition & 2 deletions batching/src/main/java/com/flipkart/batching/BatchManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
import com.flipkart.batching.toolbox.LogUtil;

import java.util.Collection;
import java.util.HashSet;
import java.util.Set;

/**
* BatchManager that implements {@link BatchController} interface. BatchManager uses builder pattern
Expand Down Expand Up @@ -202,5 +200,6 @@ public Builder enableLogging() {
public BatchManager<E, T> build(Context context) {
return new BatchManager<>(this, context);
}

}
}
5 changes: 1 addition & 4 deletions batching/src/main/java/com/flipkart/batching/TagBatchManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;

/**
* TagBatchManager class that extends {@link BatchController}.
Expand All @@ -53,13 +51,12 @@ public class TagBatchManager<E extends Data, T extends Batch<E>> implements Batc
SerializationStrategy<E, T> serializationStrategy;
TagBatchingStrategy<TagData> tagBatchingStrategy;
TagBatchReadyListener<TagData> tagBatchReadyListener;
private ArrayList<TagInfo> tagParametersList = new ArrayList<>();

protected TagBatchManager(Builder builder, final Context context) {
tagBatchingStrategy = new TagBatchingStrategy<>();
tagBatchReadyListener = new TagBatchReadyListener<>();

tagParametersList = builder.getTagInfoList();
ArrayList<TagInfo> tagParametersList = builder.getTagInfoList();

for (int i = 0; i < tagParametersList.size(); i++) {
tagBatchReadyListener.addListenerForTag(tagParametersList.get(i).tag, tagParametersList.get(i).tagBatchReadyListener);
Expand Down
14 changes: 6 additions & 8 deletions ...hing/src/main/java/com/flipkart/batching/listener/NetworkPersistedBatchReadyListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,10 @@ public class NetworkPersistedBatchReadyListener<E extends Data, T extends Batch<
int maxRetryCount;
boolean needsResumeOnReady = false;
boolean waitingForCallback = false;
boolean receiverRegistered;
boolean callFinishAfterMaxRetry = false;
private NetworkBatchListener<E, T> networkBatchListener;
private Context context;
private NetworkBroadcastReceiver networkBroadcastReceiver = new NetworkBroadcastReceiver();
private NetworkBroadcastReceiver networkBroadcastReceiver;
private PersistedBatchCallback<T> persistedBatchCallback = new PersistedBatchCallback<T>() {
@Override
public void onPersistFailure(T batch, Exception e) {
Expand Down Expand Up @@ -119,19 +118,19 @@ public void setDefaultBackoffMultiplier(float defaultBackoffMultiplier) {
}

void unregisterReceiver() {
if (receiverRegistered) {
if (null != networkBroadcastReceiver) {
context.unregisterReceiver(networkBroadcastReceiver);
receiverRegistered = false;
networkBroadcastReceiver = null;
LogUtil.log(TAG, "Unregistered network broadcast receiver {}" + this);
}
}

void registerReceiverIfRequired() {
if (!receiverRegistered) {
if (null == networkBroadcastReceiver) {
//Register the broadcast receiver
networkBroadcastReceiver = new NetworkBroadcastReceiver();
IntentFilter filter = new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION);
context.registerReceiver(networkBroadcastReceiver, filter);
receiverRegistered = true;
LogUtil.log(TAG, "Registered network broadcast receiver {}" + this);
}
}
Expand Down Expand Up @@ -217,9 +216,8 @@ private void resetRetryCounters() {
* @return new timeOut
*/
int exponentialBackOff() {
int timeOut = mCurrentTimeoutMs;
mCurrentTimeoutMs = (int) (mCurrentTimeoutMs + (mCurrentTimeoutMs * defaultBackoffMultiplier));
return timeOut;
return mCurrentTimeoutMs;
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions batching/src/main/java/com/flipkart/batching/listener/PersistedBatchReadyListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,12 @@ public void finish(final T batch) {
handler.post(new Runnable() {
@Override
public void run() {
if (queueFile.size() > 0) {
int queueSize = queueFile.size();
if (queueSize > 0) {
try {
if (peekedBatch != null) {
if (batch != null && batch.equals(peekedBatch)) {
boolean isCached = cachedQueue.size() == queueFile.size();
boolean isCached = cachedQueue.size() == queueSize;
queueFile.remove();
if (isCached) {
cachedQueue.remove();
Expand Down
2 changes: 1 addition & 1 deletion batching/src/main/java/com/flipkart/batching/persistence/DatabaseHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public Collection<E> getAllData() throws IOException {
* @param dataCollection collection of {@link Data} objects to be deleted
*/
public void deleteDataList(Collection<E> dataCollection) {
List<String> eventIdList = new ArrayList<>();
List<String> eventIdList = new ArrayList<>(dataCollection.size());
for (E data : dataCollection) {
eventIdList.add(String.valueOf(data.getEventId()));
}
Expand Down
7 changes: 2 additions & 5 deletions batching/src/main/java/com/flipkart/batching/persistence/TagBasedPersistenceStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,12 @@ public class TagBasedPersistenceStrategy<E extends Data> implements PersistenceS
public TagBasedPersistenceStrategy(Tag tag, PersistenceStrategy<E> persistenceStrategy) {
if (tag == null) {
throw new IllegalArgumentException("Tag cannot be null");
} else {
this.tag = tag;
}

if (persistenceStrategy == null) {
throw new IllegalArgumentException("PersistenceStrategy cannot be null");
} else {
this.persistenceStrategy = persistenceStrategy;
}
this.tag = tag;
this.persistenceStrategy = persistenceStrategy;
}

@Override
Expand Down
5 changes: 1 addition & 4 deletions batching/src/main/java/com/flipkart/batching/tape/InMemoryObjectQueue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ public void remove() {
@Override
public void remove(int n) {
for (int i = 0; i < n; i++) {
tasks.remove();
if (listener != null) {
listener.onRemove(this);
}
remove();
}
}

Expand Down
32 changes: 16 additions & 16 deletions batching/src/test/java/com/flipkart/batching/listener/NetworkPersistedBatchReadyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,19 @@ public void test5XXRetryPolicy() throws IOException {
//verify that it gets called once
verify(networkBatchListener, times(1)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
//verify that it gets called 2 times
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs());
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
verify(networkBatchListener, times(2)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
//verify that it gets called 3 times
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4);
verify(networkBatchListener, times(3)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
//verify that it gets called 4 times
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 8);
verify(networkBatchListener, times(4)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
//verify that it gets called 5 times
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 8);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 16);
verify(networkBatchListener, times(5)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
//verify that it does not gets called after 5 times
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 16);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 32);

ArgumentCaptor<ValueCallback> valueCallbackCapture = ArgumentCaptor.forClass(ValueCallback.class);
verify(networkBatchListener, times(5)).performNetworkRequest(any(Batch.class), valueCallbackCapture.capture());
Expand All @@ -141,7 +141,7 @@ public void test5XXRetryPolicy() throws IOException {
//verify that it gets called once
verify(networkBatchListener, times(6)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
//verify that it gets called 2 times
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs());
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
verify(networkBatchListener, times(7)).performNetworkRequest(any(Batch.class), any(ValueCallback.class));
}

Expand Down Expand Up @@ -338,18 +338,18 @@ public void testRetryPolicy() {

//verify that it gets called 1 times
verify(networkBatchListener, times(1)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs());
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
//verify that it gets called 2 times after waiting for specified time
verify(networkBatchListener, times(2)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4);
//verify that it gets called 3 times after waiting for specified time
verify(networkBatchListener, times(3)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
//verify that it gets called 3 times after waiting for specified time
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 8);
//verify that it gets called 4 times after waiting for specified time
verify(networkBatchListener, times(4)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
sendFakeNetworkBroadcast(context);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 8);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 16);
// now it should have stopped retrying anymore since max retry is reached
verify(networkBatchListener, times(4)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));

Expand All @@ -360,14 +360,14 @@ public void testRetryPolicy() {
shadowLooper.idle();
// note : now flow will get resumed, which mean network request for first batch is retried (NOT second batch)
verify(networkBatchListener, times(5)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs());
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
sendFakeNetworkBroadcast(context);
verify(networkBatchListener, times(6)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));

requestResponse.complete = true;
requestResponse.httpErrorCode = 200;
sendFakeNetworkBroadcast(context);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2); // this will call second batch
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4); // this will call second batch
//verify that it gets called 1 time with new batch
verify(networkBatchListener, times(1)).performNetworkRequest(eq(secondBatch), any(ValueCallback.class));
shadowLooper.runToEndOfTasks();
Expand Down Expand Up @@ -401,17 +401,17 @@ public void testFinishCalledIfRemoveAfterMaxRetryTrue() {

//verify that it gets called 1 times
verify(networkBatchListener, times(1)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs());
shadowLooper.idle(networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
//verify that it gets called 2 times after waiting for specified time
verify(networkBatchListener, times(2)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 2);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4);
//verify that it gets called 3 times after waiting for specified time
verify(networkBatchListener, times(3)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
//verify that it gets called 3 times after waiting for specified time
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 4);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 8);
//verify that it gets called 4 times after waiting for specified time
verify(networkBatchListener, times(4)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 8);
shadowLooper.idle(callbackIdle + networkPersistedBatchReadyListener.getDefaultTimeoutMs() * 16);
// now it should have stopped retrying anymore since max retry is reached
verify(networkBatchListener, times(4)).performNetworkRequest(eq(firstBatch), any(ValueCallback.class));

Expand Down

0 comments on commit 8ef1113

Please sign in to comment.