Modifies OperationsService so that activities can request the resulf of operations finished while the activity was 'rotating'; and applie to DetectAuthenticationMethodOperation

src/com/owncloud/android/authentication/AuthenticatorActivity.java

@@ -45,6 +45,7 @@ import android.support.v4.app.FragmentTransaction;
 import android.text.Editable;
 import android.text.InputType;
 import android.text.TextWatcher;
+import android.util.Log;
 import android.view.KeyEvent;
 import android.view.MotionEvent;
 import android.view.View;
@@ -124,6 +125,9 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
     private static final String KEY_AUTH_STATUS_ICON = "AUTH_STATUS_ICON";
     private static final String KEY_REFRESH_BUTTON_ENABLED = "KEY_REFRESH_BUTTON_ENABLED";
     //private static final String KEY_IS_SHARED_SUPPORTED = "KEY_IS_SHARE_SUPPORTED";
+    private static final String KEY_SERVER_AUTH_METHOD = "KEY_SERVER_AUTH_METHOD";
+    private static final String KEY_DETECT_AUTH_OP_ID = "KEY_DETECT_AUTH_OP_ID";
+
 
     private static final String AUTH_ON = "on";
     private static final String AUTH_OFF = "off";
@@ -144,12 +148,12 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
     private String mAuthMessageText;
     private int mAuthMessageVisibility, mServerStatusText, mServerStatusIcon;
     private boolean mServerIsChecked, mServerIsValid, mIsSslConn;
+    private AuthenticationMethod mServerAuthMethod = AuthenticationMethod.UNKNOWN;
+    private int mDetectAuthOpId = -1;
+
     private int mAuthStatusText, mAuthStatusIcon;    
     private TextView mAuthStatusLayout;
 
-    private ServiceConnection mOperationsConnection = null;
-    private OperationsServiceBinder mOperationsBinder = null;
-    
     private final Handler mHandler = new Handler();
     private Thread mOperationThread;
     private GetRemoteStatusOperation mOcServerChkOperation;
@@ -192,7 +196,6 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
     
     private OperationsServiceBinder mOperationsServiceBinder = null;
 
-
     /**
      * {@inheritDoc}
      * 
@@ -204,31 +207,17 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
         getWindow().requestFeature(Window.FEATURE_NO_TITLE);
 
         // bind to Operations Service
-        mOperationsConnection = new ServiceConnection() {
-
-            @Override
-            public void onServiceConnected(ComponentName name, IBinder service) {
-                Log_OC.d(TAG, "Operations service connected");
-                mOperationsBinder = (OperationsServiceBinder) service;
-            }
-
-            @Override
-            public void onServiceDisconnected(ComponentName name) {
-                Log_OC.d(TAG, "Operations service crashed");
-                mOperationsBinder = null;
-            }
-            
-        };
+        mOperationsServiceConnection = new OperationsServiceConnection();
         if (!bindService(new Intent(this, OperationsService.class), 
-                        mOperationsConnection, 
-                        Context.BIND_AUTO_CREATE)) {
+                mOperationsServiceConnection, 
+                Context.BIND_AUTO_CREATE)) {
             Toast.makeText(this, 
                     R.string.error_cant_bind_to_operations_service, 
                     Toast.LENGTH_LONG)
                         .show();
             finish();
         }
-
+                
         /// set view and get references to view elements
         setContentView(R.layout.account_setup);
         mAuthMessage = (TextView) findViewById(R.id.auth_message);
@@ -347,6 +336,10 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
             refreshButtonEnabled = savedInstanceState.getBoolean(KEY_REFRESH_BUTTON_ENABLED);
 
 
+            mServerAuthMethod = AuthenticationMethod.valueOf(
+                    savedInstanceState.getString(KEY_SERVER_AUTH_METHOD));
+            mDetectAuthOpId = savedInstanceState.getInt(KEY_DETECT_AUTH_OP_ID);
+
         }
 
         if (mAuthMessageVisibility== View.VISIBLE) {
@@ -433,8 +426,6 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
             }
         });
         
-        mOperationsServiceConnection = new OperationsServiceConnection();
-        bindService(new Intent(this, OperationsService.class), mOperationsServiceConnection, Context.BIND_AUTO_CREATE);
     }
 
 
@@ -487,6 +478,7 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
      */
     @Override
     protected void onSaveInstanceState(Bundle outState) {
+        //Log.wtf(TAG, "onSaveInstanceState init" );
         super.onSaveInstanceState(outState);
 
         /// connection state and info
@@ -517,7 +509,10 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
 
         // refresh button enabled
         outState.putBoolean(KEY_REFRESH_BUTTON_ENABLED, (mRefreshButton.getVisibility() == View.VISIBLE));
-
+        
+        outState.putString(KEY_SERVER_AUTH_METHOD, mServerAuthMethod.name());
+        outState.putInt(KEY_DETECT_AUTH_OP_ID, mDetectAuthOpId);
+        //Log.wtf(TAG, "onSaveInstanceState end" );
     }
 
 
@@ -538,33 +533,15 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
     }
 
 
-    @Override 
-    protected void onStart() {
-        if (mOperationsServiceBinder != null) {
-            mOperationsServiceBinder.addOperationListener(AuthenticatorActivity.this, mHandler);
-        }
-        
-        super.onStart();
-    }
-    
-    
-    @Override 
-    protected void onStop() {
-        if (mOperationsServiceBinder != null) {
-            mOperationsServiceBinder.removeOperationListener(this);
-        }
-        super.onStop();
-    }
-    
-
-
     /**
      * The redirection triggered by the OAuth authentication server as response to the GET AUTHORIZATION, and 
      * deferred in {@link #onNewIntent(Intent)}, is processed here.
      */
     @Override
     protected void onResume() {
+        //Log.wtf(TAG, "onResume init" );
         super.onResume();
+        
         if (mAction == ACTION_UPDATE_TOKEN && mJustCreated && getIntent().getBooleanExtra(EXTRA_ENFORCED_UPDATE, false)) {
             if (AccountTypeUtils.getAuthTokenTypeAccessToken(MainApp.getAccountType()).equals(mAuthTokenType)) {
                 //Toast.makeText(this, R.string.auth_expired_oauth_token_toast, Toast.LENGTH_LONG).show();
@@ -581,17 +558,33 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
         if (mNewCapturedUriFromOAuth2Redirection != null) {
             getOAuth2AccessTokenFromCapturedRedirection();            
         }
-
+        
         mJustCreated = false;
 
+        if (mOperationsServiceBinder != null) {
+            doOnResumeAndBound();
+        }
+        
+        //Log.wtf(TAG, "onResume end" );
     }
+
     
+    @Override
+    protected void onPause() {
+        //Log.wtf(TAG, "onPause init" );
+        if (mOperationsServiceBinder != null) {
+            //Log.wtf(TAG, "unregistering to listen for operation callbacks" );
+            mOperationsServiceBinder.removeOperationListener(this);
+        }
+        super.onPause();
+        //Log.wtf(TAG, "onPause end" );
+    }
     
     @Override
     protected void onDestroy() {
-        if (mOperationsConnection != null) {
-            unbindService(mOperationsConnection);
-            mOperationsBinder = null;
+        if (mOperationsServiceConnection != null) {
+            unbindService(mOperationsServiceConnection);
+            mOperationsServiceBinder = null;
         }
         super.onDestroy();
     }
@@ -885,13 +878,15 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
             onGetUserNameFinish((GetRemoteUserNameOperation) operation, result);
 
         } else if (operation instanceof DetectAuthenticationMethodOperation) {
-            onDetectAutheticationFinish((DetectAuthenticationMethodOperation) operation, result);
+            Log.wtf(TAG, "received detection response through callback" );
+            onDetectAuthenticationFinish(result);
         }
 
     }
 
-    private void onDetectAutheticationFinish(DetectAuthenticationMethodOperation operation, RemoteOperationResult result) {
+    private void onDetectAuthenticationFinish(RemoteOperationResult result) {
         // Read authentication method
+        mDetectAuthOpId = -1;
         if (result.getData().size() > 0) {
             AuthenticationMethod authMethod = (AuthenticationMethod) result.getData().get(0);
             String basic = AccountTypeUtils.getAuthTokenTypePass(MainApp.getAccountType());
@@ -1037,11 +1032,19 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
         String webdav_path = AccountUtils.getWebdavPath(mDiscoveredVersion, mAuthTokenType);
         
         /// test credentials 
-        Intent service = new Intent(this, OperationsService.class);        
-        service.setAction(OperationsService.ACTION_DETECT_AUTHENTICATION_METHOD);
-        service.putExtra(OperationsService.EXTRA_SERVER_URL, mHostBaseUrl);
-        service.putExtra(OperationsService.EXTRA_WEBDAV_PATH, webdav_path);
-        startService(service);
+        //Intent detectAuthIntent = new Intent(this, OperationsService.class);
+        Intent detectAuthIntent = new Intent();
+        detectAuthIntent.setAction(OperationsService.ACTION_DETECT_AUTHENTICATION_METHOD);
+        detectAuthIntent.putExtra(OperationsService.EXTRA_SERVER_URL, mHostBaseUrl);
+        detectAuthIntent.putExtra(OperationsService.EXTRA_WEBDAV_PATH, webdav_path);
+        
+        //if (mOperationsBinder != null) {  // let's let it crash to detect if is really possible
+        mServerAuthMethod = AuthenticationMethod.UNKNOWN;
+        if (mOperationsServiceBinder != null) {
+            //Log.wtf(TAG, "starting detection..." );
+            mDetectAuthOpId = mOperationsServiceBinder.newOperation(detectAuthIntent);
+        }
+        //}
     }
 
 
@@ -1866,18 +1869,35 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
         }
 
     }
-
+    
+    
+    private void doOnResumeAndBound() {
+        //Log.wtf(TAG, "registering to listen for operation callbacks" );
+        mOperationsServiceBinder.addOperationListener(AuthenticatorActivity.this, mHandler);
+        
+        if (mDetectAuthOpId != -1) {
+            RemoteOperationResult result = 
+                    mOperationsServiceBinder.getOperationResultIfFinished(mDetectAuthOpId);
+            if (result != null) {
+                //Log.wtf(TAG, "found result of operation finished while rotating");
+                onDetectAuthenticationFinish(result);
+            }
+        }
+    }
+    
     /** 
-     * Implements callback methods for service binding. Passed as a parameter to { 
+     * Implements callback methods for service binding. 
      */
     private class OperationsServiceConnection implements ServiceConnection {
 
         @Override
         public void onServiceConnected(ComponentName component, IBinder service) {
             if (component.equals(new ComponentName(AuthenticatorActivity.this, OperationsService.class))) {
-                Log_OC.d(TAG, "Operations service connected");
+                //Log_OC.wtf(TAG, "Operations service connected");
                 mOperationsServiceBinder = (OperationsServiceBinder) service;
-                mOperationsServiceBinder.addOperationListener(AuthenticatorActivity.this, mHandler);
+                
+                doOnResumeAndBound();
+                
             } else {
                 return;
             }
@@ -1887,11 +1907,11 @@ SsoWebViewClientListener, OnSslUntrustedCertListener {
         @Override
         public void onServiceDisconnected(ComponentName component) {
             if (component.equals(new ComponentName(AuthenticatorActivity.this, OperationsService.class))) {
-                Log_OC.d(TAG, "Operations service disconnected");
+                Log_OC.e(TAG, "Operations service crashed");
                 mOperationsServiceBinder = null;
-                // TODO whatever could be waiting for the service is unbound
             }
         }
     
     }
+    
 }

src/com/owncloud/android/services/OperationsService.java

@@ -18,10 +18,10 @@
 package com.owncloud.android.services;
 
 import java.io.IOException;
-import java.util.HashMap;
 import java.util.Iterator;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ConcurrentMap;
 
 import com.owncloud.android.datamodel.FileDataStorageManager;
 import com.owncloud.android.lib.common.OwnCloudClientFactory;
@@ -68,7 +68,11 @@ public class OperationsService extends Service {
     public static final String ACTION_OPERATION_ADDED = OperationsService.class.getName() + ".OPERATION_ADDED";
     public static final String ACTION_OPERATION_FINISHED = OperationsService.class.getName() + ".OPERATION_FINISHED";
 
-    private ConcurrentLinkedQueue<Pair<Target, RemoteOperation>> mPendingOperations = new ConcurrentLinkedQueue<Pair<Target, RemoteOperation>>();
+    private ConcurrentLinkedQueue<Pair<Target, RemoteOperation>> mPendingOperations = 
+            new ConcurrentLinkedQueue<Pair<Target, RemoteOperation>>();
+    
+    private ConcurrentMap<Integer, RemoteOperationResult> mOperationResults =
+            new ConcurrentHashMap<Integer, RemoteOperationResult>();
     
     private static class Target {
         public Uri mServerUrl = null;
@@ -101,6 +105,7 @@ public class OperationsService extends Service {
         mBinder = new OperationsServiceBinder();
     }
 
+    
     /**
      * Entry point to add a new operation to the queue of operations.
      * 
@@ -112,51 +117,11 @@ public class OperationsService extends Service {
      */
     @Override
     public int onStartCommand(Intent intent, int flags, int startId) {
-        if (!intent.hasExtra(EXTRA_ACCOUNT) && !intent.hasExtra(EXTRA_SERVER_URL)) {
-            Log_OC.e(TAG, "Not enough information provided in intent");
-            return START_NOT_STICKY;
-        }
-        try {
-            Account account = intent.getParcelableExtra(EXTRA_ACCOUNT);
-            String serverUrl = intent.getStringExtra(EXTRA_SERVER_URL);
-            
-            Target target = new Target(account, (serverUrl == null) ? null : Uri.parse(serverUrl));
-            RemoteOperation operation = null;
-            
-            String action = intent.getAction();
-            if (action.equals(ACTION_CREATE_SHARE)) {  // Create Share
-                String remotePath = intent.getStringExtra(EXTRA_REMOTE_PATH);
-                Intent sendIntent = intent.getParcelableExtra(EXTRA_SEND_INTENT);
-                if (remotePath.length() > 0) {
-                    operation = new CreateShareOperation(remotePath, ShareType.PUBLIC_LINK, 
-                            "", false, "", 1, sendIntent);
-                }
-            } else if (action.equals(ACTION_UNSHARE)) {  // Unshare file
-                String remotePath = intent.getStringExtra(EXTRA_REMOTE_PATH);
-                if (remotePath.length() > 0) {
-                    operation = new UnshareLinkOperation(remotePath, this.getApplicationContext());
-                }
-            } else if (action.equals(ACTION_DETECT_AUTHENTICATION_METHOD)) { // Detect Authentication Method
-                String webdav_url = serverUrl + intent.getStringExtra(EXTRA_WEBDAV_PATH);
-                operation = new DetectAuthenticationMethodOperation(this.getApplicationContext(), webdav_url);
-            
-            } else {
-                // nothing we are going to handle
-                return START_NOT_STICKY;
-            }
-            
-            mPendingOperations.add(new Pair<Target , RemoteOperation>(target, operation));
-            //sendBroadcastNewOperation(target, operation);
-            
-            Message msg = mServiceHandler.obtainMessage();
-            msg.arg1 = startId;
-            mServiceHandler.sendMessage(msg);
-            
-        } catch (IllegalArgumentException e) {
-            Log_OC.e(TAG, "Bad information provided in intent: " + e.getMessage());
-            return START_NOT_STICKY;
-        }
-        
+        //Log.wtf(TAG, "onStartCommand init" );
+        Message msg = mServiceHandler.obtainMessage();
+        msg.arg1 = startId;
+        mServiceHandler.sendMessage(msg);
+        //Log.wtf(TAG, "onStartCommand end" );
         return START_NOT_STICKY;
     }
 
@@ -167,6 +132,7 @@ public class OperationsService extends Service {
      */
     @Override
     public IBinder onBind(Intent intent) {
+        //Log.wtf(TAG, "onBind" );
         return mBinder;
     }
 
@@ -176,7 +142,7 @@ public class OperationsService extends Service {
      */
     @Override
     public boolean onUnbind(Intent intent) {
-        //((OperationsServiceBinder)mBinder).clearListeners();
+        ((OperationsServiceBinder)mBinder).clearListeners();
         return false;   // not accepting rebinding (default behaviour)
     }
 
@@ -191,7 +157,8 @@ public class OperationsService extends Service {
         /** 
          * Map of listeners that will be reported about the end of operations from a {@link OperationsServiceBinder} instance 
          */
-        private Map<OnRemoteOperationListener, Handler> mBoundListeners = new HashMap<OnRemoteOperationListener, Handler>();
+        private ConcurrentMap<OnRemoteOperationListener, Handler> mBoundListeners = 
+                new ConcurrentHashMap<OnRemoteOperationListener, Handler>();
         
         /**
          * Cancels an operation
@@ -216,7 +183,9 @@ public class OperationsService extends Service {
          * @param callbackHandler   {@link Handler} to access the listener without breaking Android threading protection.
          */
         public void addOperationListener (OnRemoteOperationListener listener, Handler callbackHandler) {
-            mBoundListeners.put(listener, callbackHandler);
+            synchronized (mBoundListeners) {
+                mBoundListeners.put(listener, callbackHandler);
+            }
         }
         
         
@@ -226,7 +195,9 @@ public class OperationsService extends Service {
          * @param listener      Object to notify about progress of transfer.    
          */
         public void removeOperationListener (OnRemoteOperationListener listener) {
-            mBoundListeners.remove(listener);
+            synchronized (mBoundListeners) {
+                mBoundListeners.remove(listener);
+            }
         }
 
 
@@ -239,6 +210,74 @@ public class OperationsService extends Service {
             return (!mPendingOperations.isEmpty());
         }
 
+
+        /**
+         * Creates and adds to the queue a new operation, as described by operationIntent
+         * 
+         * @param operationIntent       Intent describing a new operation to queue and execute.
+         * @return                      Identifier of the operation created, or -1 if failed.
+         */
+        public int newOperation(Intent operationIntent) {
+            RemoteOperation operation = null;
+            Target target = null;
+            try {
+                if (!operationIntent.hasExtra(EXTRA_ACCOUNT) && 
+                        !operationIntent.hasExtra(EXTRA_SERVER_URL)) {
+                    Log_OC.e(TAG, "Not enough information provided in intent");
+                    
+                } else {
+                    Account account = operationIntent.getParcelableExtra(EXTRA_ACCOUNT);
+                    String serverUrl = operationIntent.getStringExtra(EXTRA_SERVER_URL);
+                    target = new Target(
+                            account, 
+                            (serverUrl == null) ? null : Uri.parse(serverUrl)
+                    );
+                    
+                    String action = operationIntent.getAction();
+                    if (action.equals(ACTION_CREATE_SHARE)) {  // Create Share
+                        String remotePath = operationIntent.getStringExtra(EXTRA_REMOTE_PATH);
+                        Intent sendIntent = operationIntent.getParcelableExtra(EXTRA_SEND_INTENT);
+                        if (remotePath.length() > 0) {
+                            operation = new CreateShareOperation(remotePath, ShareType.PUBLIC_LINK, 
+                                    "", false, "", 1, sendIntent);
+                        }
+                    } else if (action.equals(ACTION_UNSHARE)) {  // Unshare file
+                        String remotePath = operationIntent.getStringExtra(EXTRA_REMOTE_PATH);
+                        if (remotePath.length() > 0) {
+                            operation = new UnshareLinkOperation(
+                                    remotePath, 
+                                    OperationsService.this);
+                        }
+                    } else if (action.equals(ACTION_DETECT_AUTHENTICATION_METHOD)) { 
+                        // Detect Authentication Method
+                        String webdav_url = 
+                                serverUrl + operationIntent.getStringExtra(EXTRA_WEBDAV_PATH);
+                        operation = new DetectAuthenticationMethodOperation(
+                                OperationsService.this, 
+                                webdav_url);
+                    }
+                }
+                    
+            } catch (IllegalArgumentException e) {
+                Log_OC.e(TAG, "Bad information provided in intent: " + e.getMessage());
+                operation = null;
+            }
+
+            if (operation != null) {
+                mPendingOperations.add(new Pair<Target , RemoteOperation>(target, operation));
+                startService(new Intent(OperationsService.this, OperationsService.class));
+                return operation.hashCode();
+                
+            } else {
+                return -1;
+            }
+        }
+
+        public RemoteOperationResult getOperationResultIfFinished(int mDetectAuthOpId) {
+            //Log_OC.wtf(TAG, "Searching result for operation with id " + mDetectAuthOpId);
+            return mOperationResults.remove(mDetectAuthOpId);
+        }
+
     }
     
     
@@ -271,6 +310,8 @@ public class OperationsService extends Service {
      */
     private void nextOperation() {
         
+        //Log.wtf(TAG, "nextOperation init" );
+        
         Pair<Target, RemoteOperation> next = null;
         synchronized(mPendingOperations) {
             next = mPendingOperations.peek();
@@ -326,6 +367,7 @@ public class OperationsService extends Service {
             } finally {
                 synchronized(mPendingOperations) {
                     mPendingOperations.poll();
+                    mOperationResults.put(mCurrentOperation.hashCode(), result);
                 }
             }
             
@@ -388,7 +430,9 @@ public class OperationsService extends Service {
      * @param operation         Finished operation.
      * @param result            Result of the operation.
      */
-    private void callbackOperationListeners(Target target, final RemoteOperation operation, final RemoteOperationResult result) {
+    private void callbackOperationListeners(
+            Target target, final RemoteOperation operation, final RemoteOperationResult result) {
+        int count = 0;
         Iterator<OnRemoteOperationListener> listeners = mBinder.mBoundListeners.keySet().iterator();
         while (listeners.hasNext()) {
             final OnRemoteOperationListener listener = listeners.next();
@@ -400,9 +444,10 @@ public class OperationsService extends Service {
                         listener.onRemoteOperationFinish(operation, result);
                     }
                 });
+                count += 1;
             }
         }
-            
+        Log_OC.d(TAG, "Called " + count + " listeners");
     }