From c6cd21cdf0a31d9106653f20d78e436794ceb64a Mon Sep 17 00:00:00 2001 From: Severiano Jaramillo Date: Tue, 6 Nov 2018 11:36:11 -0600 Subject: [PATCH 1/2] Fix crash in NetworkService due to components other than NetworkServiceManager binding to the service without the proper initialization information. This initialization was moved to another method named bootstrapService which is only called from NetworkServiceManager after the NetworkService has been properly connected. --- .../graphenej/api/android/NetworkService.java | 15 ++++-- .../api/android/NetworkServiceManager.java | 54 +++++++++++-------- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java b/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java index 9caea18..121c9eb 100644 --- a/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java +++ b/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java @@ -81,7 +81,7 @@ public class NetworkService extends Service { public static final int NORMAL_CLOSURE_STATUS = 1000; // Time to wait before retrying a connection attempt - private final int DEFAULT_RETRY_DELAY = 500; + private static final int DEFAULT_RETRY_DELAY = 500; // Default connection delay when using the node latency verification strategy. This initial // delay is required in order ot make sure we have a fair selection of node latencies from @@ -284,7 +284,17 @@ public class NetworkService extends Service { @Nullable @Override public IBinder onBind(Intent intent) { - Bundle extras = intent.getExtras(); + return mBinder; + } + + /** + * Initialize information and try to connect to a node accordingly. This methods were moved + * from onBind to avoid crashes due to components other than {@link NetworkServiceManager} + * binding to the service without submitting the proper information. + * + * @param extras Bundle that contains all required information for a proper initialization + */ + public void bootstrapService(Bundle extras) { // Retrieving credentials and requested API data from the shared preferences mUsername = extras.getString(NetworkService.KEY_USERNAME, ""); mPassword = extras.getString(NetworkService.KEY_PASSWORD, ""); @@ -327,7 +337,6 @@ public class NetworkService extends Service { mHandler.postDelayed(mConnectAttempt, DEFAULT_INITIAL_DELAY); } } - return mBinder; } /** diff --git a/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkServiceManager.java b/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkServiceManager.java index 3d31f9b..c64aa1b 100644 --- a/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkServiceManager.java +++ b/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkServiceManager.java @@ -25,14 +25,14 @@ import cy.agorise.graphenej.stats.ExponentialMovingAverage; */ public class NetworkServiceManager implements Application.ActivityLifecycleCallbacks { - private final static String TAG = "NetworkServiceManager"; + private final String TAG = this.getClass().getName(); /** * Constant used to specify how long will the app wait for another activity to go through its starting life * cycle events before running the teardownConnectionTask task. * * This is used as a means to detect whether or not the user has left the app. */ - private final int DISCONNECT_DELAY = 1500; + private static final int DISCONNECT_DELAY = 1500; /** * Handler instance used to schedule tasks back to the main thread @@ -72,8 +72,8 @@ public class NetworkServiceManager implements Application.ActivityLifecycleCallb } }; - public NetworkServiceManager(Context context){ - mContextReference = new WeakReference(context); + private NetworkServiceManager(Context context){ + mContextReference = new WeakReference<>(context); } @Override @@ -89,27 +89,37 @@ public class NetworkServiceManager implements Application.ActivityLifecycleCallb // Creating a new Intent that will be used to start the NetworkService Context context = mContextReference.get(); Intent intent = new Intent(context, NetworkService.class); - - // Adding user-provided node URLs - StringBuilder stringBuilder = new StringBuilder(); - Iterator it = mCustomNodeUrls.iterator(); - while(it.hasNext()){ - stringBuilder.append(it.next()); - if(it.hasNext()) stringBuilder.append(","); - } - String customNodes = stringBuilder.toString(); - - // Adding all - intent.putExtra(NetworkService.KEY_USERNAME, mUserName) - .putExtra(NetworkService.KEY_PASSWORD, mPassword) - .putExtra(NetworkService.KEY_REQUESTED_APIS, mRequestedApis) - .putExtra(NetworkService.KEY_NODE_URLS, customNodes) - .putExtra(NetworkService.KEY_AUTO_CONNECT, mAutoConnect) - .putExtra(NetworkService.KEY_ENABLE_LATENCY_VERIFIER, mVerifyLatency); context.bindService(intent, mServiceConnection, Context.BIND_AUTO_CREATE); } } + /** + * This method passes all the required information to the NetworkService to properly + * initialize itself + */ + private void passRequiredInfoToConfigureService() { + // Adding user-provided node URLs + StringBuilder stringBuilder = new StringBuilder(); + Iterator it = mCustomNodeUrls.iterator(); + while(it.hasNext()){ + stringBuilder.append(it.next()); + if(it.hasNext()) stringBuilder.append(","); + } + String customNodes = stringBuilder.toString(); + + Bundle b = new Bundle(); + + // Adding all + b.putString(NetworkService.KEY_USERNAME, mUserName); + b.putString(NetworkService.KEY_PASSWORD, mPassword); + b.putInt(NetworkService.KEY_REQUESTED_APIS, mRequestedApis); + b.putString(NetworkService.KEY_NODE_URLS, customNodes); + b.putBoolean(NetworkService.KEY_AUTO_CONNECT, mAutoConnect); + b.putBoolean(NetworkService.KEY_ENABLE_LATENCY_VERIFIER, mVerifyLatency); + + mService.bootstrapService(b); + } + @Override public void onActivityPaused(Activity activity) { mHandler.postDelayed(mDisconnectRunnable, DISCONNECT_DELAY); @@ -133,6 +143,8 @@ public class NetworkServiceManager implements Application.ActivityLifecycleCallb // We've bound to LocalService, cast the IBinder and get LocalService instance NetworkService.LocalBinder binder = (NetworkService.LocalBinder) service; mService = binder.getService(); + + passRequiredInfoToConfigureService(); } @Override From 214891fcc94bf0e06fb006f77c63fed167b71cd1 Mon Sep 17 00:00:00 2001 From: Severiano Jaramillo Date: Wed, 7 Nov 2018 16:22:29 -0600 Subject: [PATCH 2/2] Added proper methods to deal with bitshares nodes that send 'trash info', which are nodes that potentially do not have installed the history plugin and thus do not respond properly to history related requests. As this information is needed for PalmPay we cannot rely on such nodes and have to try to connect to a different node. --- .../graphenej/api/android/NetworkService.java | 104 ++++++++++++------ 1 file changed, 70 insertions(+), 34 deletions(-) diff --git a/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java b/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java index 7a2f2db..2b2f7a2 100644 --- a/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java +++ b/graphenej/src/main/java/cy/agorise/graphenej/api/android/NetworkService.java @@ -80,6 +80,7 @@ public class NetworkService extends Service { private final String TAG = this.getClass().getName(); public static final int NORMAL_CLOSURE_STATUS = 1000; + private static final int NO_HISTORY_CLOSURE_STATUS = 1001; // Time to wait before retrying a connection attempt private static final int DEFAULT_RETRY_DELAY = 500; @@ -565,6 +566,9 @@ public class NetworkService extends Service { } else if (requestClass == GetFullAccounts.class) { Type GetFullAccountsResponse = new TypeToken>>(){}.getType(); parsedResponse = gson.fromJson(text, GetFullAccountsResponse); + + if(parsedResponse != null) + verifyNodeHasHistoryApi(parsedResponse); } else if(requestClass == GetKeyReferences.class){ Type GetKeyReferencesResponse = new TypeToken>>>(){}.getType(); parsedResponse = gson.fromJson(text, GetKeyReferencesResponse); @@ -587,6 +591,30 @@ public class NetworkService extends Service { RxBus.getBusInstance().send(parsedResponse); } + /** + * This method inspects the node response to find out if the totalOps is equal to zero, + * in that case the current connected node may not have the history plugin so we would need + * to close the connection and choose a different node. + * + * @param parsedResponse A JSONRpcResponse from a GetFullAccounts API call + */ + private void verifyNodeHasHistoryApi(JsonRpcResponse parsedResponse) { + if(parsedResponse.result instanceof List && + ((List) parsedResponse.result).size() > 0 && + ((List) parsedResponse.result).get(0) instanceof FullAccountDetails) { + + FullAccountDetails fullAccountDetails = (FullAccountDetails) ((List) parsedResponse.result).get(0); + long totalOps = fullAccountDetails.getStatistics().total_ops; + + if (totalOps == 0) { + Log.d(TAG, "The node returned 0 total_ops for current account and may not have installed the history plugin. " + + "Trying to connect to a different node."); + + mWebSocket.close(NO_HISTORY_CLOSURE_STATUS, null); + } + } + } + /** * Private method that will just broadcast a de-serialized notification to all interested parties * @param notification De-serialized notification @@ -645,20 +673,11 @@ public class NetworkService extends Service { public void onClosed(WebSocket webSocket, int code, String reason) { super.onClosed(webSocket, code, reason); Log.d(TAG,"onClosed"); - RxBus.getBusInstance().send(new ConnectionStatusUpdate(ConnectionStatusUpdate.DISCONNECTED, ApiAccess.API_NONE)); - isLoggedIn = false; - - // Marking the selected node as not connected - mSelectedNode.setConnected(false); - - // Updating the selected node's 'connected' status on the NodeLatencyVerifier instance - if(nodeLatencyVerifier != null) - nodeLatencyVerifier.updateActiveNodeInformation(mSelectedNode); - - - // We have currently no selected node - mSelectedNode = null; + if (code == NO_HISTORY_CLOSURE_STATUS) + handleWebSocketDisconnection(true); + else + handleWebSocketDisconnection(false); } @Override @@ -669,36 +688,26 @@ public class NetworkService extends Service { for(StackTraceElement element : t.getStackTrace()){ Log.v(TAG,String.format("%s#%s:%s", element.getClassName(), element.getMethodName(), element.getLineNumber())); } - // Registering current status - isLoggedIn = false; - mCurrentId = 0; - mApiIds.clear(); // If there is a response, we print it if(response != null){ Log.e(TAG,"Response: "+response.message()); } - // Adding a very high latency value to this node in order to prevent - // us from getting it again - mSelectedNode.addLatencyValue(Long.MAX_VALUE); - nodeProvider.updateNode(mSelectedNode); + handleWebSocketDisconnection(true); + } + /** + * Method that encapsulates the behavior of handling a disconnection to the current node, and + * potentially tries to reconnect to another one. + * + * @param tryReconnection Variable that states if a reconnection to other node should be tried. + */ + private void handleWebSocketDisconnection(boolean tryReconnection) { RxBus.getBusInstance().send(new ConnectionStatusUpdate(ConnectionStatusUpdate.DISCONNECTED, ApiAccess.API_NONE)); - if(nodeProvider.getBestNode() == null){ - Log.e(TAG,"Giving up on connections"); - stopSelf(); - }else{ - Handler handler = new Handler(Looper.getMainLooper()); - handler.postDelayed(new Runnable() { - @Override - public void run() { - Log.d(TAG,"Retrying"); - connect(); - } - }, DEFAULT_RETRY_DELAY); - } + isLoggedIn = false; + // Marking the selected node as not connected mSelectedNode.setConnected(false); @@ -706,6 +715,33 @@ public class NetworkService extends Service { if(nodeLatencyVerifier != null) nodeLatencyVerifier.updateActiveNodeInformation(mSelectedNode); + if(tryReconnection) { + // Registering current status + mCurrentId = 0; + mApiIds.clear(); + + // Adding a very high latency value to this node in order to prevent + // us from getting it again + mSelectedNode.addLatencyValue(Long.MAX_VALUE); + nodeProvider.updateNode(mSelectedNode); + + RxBus.getBusInstance().send(new ConnectionStatusUpdate(ConnectionStatusUpdate.DISCONNECTED, ApiAccess.API_NONE)); + + if (nodeProvider.getBestNode() == null) { + Log.e(TAG, "Giving up on connections"); + stopSelf(); + } else { + Handler handler = new Handler(Looper.getMainLooper()); + handler.postDelayed(new Runnable() { + @Override + public void run() { + Log.d(TAG, "Retrying"); + connect(); + } + }, DEFAULT_RETRY_DELAY); + } + } + // We have currently no selected node mSelectedNode = null; }