SMSManager Context and tests (#35)

* Started work on mocks and tests.
Added SMSCore SmsManager as an optional parameter (so it can be tested)

* Fixed null reference in SMSPeerTest class
Implemented tests (using Mock objects) on some SMSHandler functions
(could no test sent and delivered listeners)
Added clearSetup() function (to be able to correctly test stuff)
Fixed NullPointerException if in setup() was passed a null context

* Improved null check using @nonnull
Improved specifications
(Code review by Luca Crema)

* Improved SMSCore specifics.
Re-added null check in SMSHandler setup() function due to a discussion in a code review.

* Improved Mock test using verify(...)
Removed useless "exception" class
(Thanks to Matteo Carnelos for code review/improvement)

* Optimized imports
Formatted code

* Removed context and useless methods
context gets now asked by the methods that uses it
improved specifications
Added and fixed unit tests

* Formatted class

* Added author Tags

Author: Cogno-Marco

smslibrary/build.gradle

@@ -29,6 +29,7 @@ dependencies {
     implementation 'com.github.CremaLuca:android-preferences:3.0'
     implementation 'androidx.appcompat:appcompat:1.1.0'
     testImplementation 'junit:junit:4.12'
+    testImplementation 'org.mockito:mockito-core:1.10.19'
     androidTestImplementation 'androidx.test.ext:junit:1.1.1'
     androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0'
     implementation 'com.github.CremaLuca:android-preferences:2.3'

smslibrary/src/main/java/com/eis/smslibrary/SMSCore.java

@@ -12,10 +12,31 @@
  * Wrapper for the {@link android.telephony} library.
  * This class is only used to interface with the core sms library of Android.
  *
- * @author Luca Crema
+ * @author Luca Crema, Marco Cognolato
  */
 final class SMSCore {
 
+    static SmsManager manager;
+
+    /**
+     * Sets up a given valid custom manager
+     * @param manager The custom manager to set up to send messages
+     *
+     * @author Marco Cognolato
+     */
+    static void setManager(SmsManager manager){
+        SMSCore.manager = manager;
+    }
+
+    /**
+     * @return Returns the pre-set up manager if not null, else returns the default manager
+     *
+     * @author Marco Cognolato
+     */
+    private static SmsManager getManager(){
+        return manager == null ? SmsManager.getDefault() : manager;
+    }
+
     /**
      * Calls the library method to send a single message
      *
@@ -25,7 +46,7 @@ final class SMSCore {
      * @param deliveredPI {@link PendingIntent} for a broadcast on message received, can be null
      */
     static void sendMessage(@NonNull final String message, @NonNull final String phoneNumber, @Nullable final PendingIntent sentPI, @Nullable final PendingIntent deliveredPI) {
-        SmsManager.getDefault().sendTextMessage(phoneNumber, null, message, sentPI, deliveredPI);
+        getManager().sendTextMessage(phoneNumber, null, message, sentPI, deliveredPI);
     }
 
     /**
@@ -37,7 +58,7 @@ static void sendMessage(@NonNull final String message, @NonNull final String pho
      * @param deliveredPIs {@link ArrayList}  of pending intents for a broadcast on message delivered, can be null
      */
     static void sendMessages(@NonNull final ArrayList<String> messages, @NonNull final String phoneNumber, @Nullable final ArrayList<PendingIntent> sentPIs, @Nullable final ArrayList<PendingIntent> deliveredPIs) {
-        SmsManager.getDefault().sendMultipartTextMessage(phoneNumber, null, messages, sentPIs, deliveredPIs);
+        getManager().sendMultipartTextMessage(phoneNumber, null, messages, sentPIs, deliveredPIs);
     }
 
 }

smslibrary/src/main/java/com/eis/smslibrary/SMSManager.java

@@ -13,18 +13,13 @@
 import com.eis.smslibrary.listeners.SMSReceivedServiceListener;
 import com.eis.smslibrary.listeners.SMSSentListener;
 
-import java.lang.ref.WeakReference;
-
 import it.lucacrema.preferences.PreferencesManager;
 
-
 /**
  * Communication handler for SMSs. It's a Singleton, you should
- * access it with {@link #getInstance}, and before doing anything you
- * should call {@link #setup}.<br/>
+ * access it with {@link #getInstance}
  *
  * @author Luca Crema, Marco Mariotto, Alberto Ursino, Marco Tommasini, Marco Cognolato
- * @since 29/11/2019
  */
 @SuppressWarnings({"WeakerAccess", "unused"})
 public class SMSManager implements CommunicationManager<SMSMessage> {
@@ -39,11 +34,9 @@ public class SMSManager implements CommunicationManager<SMSMessage> {
     private static SMSManager instance;
 
     /**
-     * Weak reference doesn't prevent garbage collector to
-     * de-allocate this class when it has reference to a
-     * context that is still running. Prevents memory leaks.
+     * Received listener reference
      */
-    private WeakReference<Context> context;
+    private SMSReceivedServiceListener receivedListener;
 
     /**
      * This message counter is used so that we can have a different action name
@@ -70,15 +63,6 @@ public static SMSManager getInstance() {
         return instance;
     }
 
-    /**
-     * Setup for the handler.
-     *
-     * @param context current context.
-     */
-    public void setup(Context context) {
-        this.context = new WeakReference<>(context);
-    }
-
     /**
      * Sends a message to a destination peer via SMS.
      * Requires {@link android.Manifest.permission#SEND_SMS}
@@ -87,7 +71,7 @@ public void setup(Context context) {
      */
     @Override
     public void sendMessage(final @NonNull SMSMessage message) {
-        sendMessage(message, null, null);
+        sendMessage(message, null, null, null);
     }
 
     /**
@@ -96,9 +80,12 @@ public void sendMessage(final @NonNull SMSMessage message) {
      *
      * @param message      to be sent in the channel to a peer
      * @param sentListener called on message sent or on error, can be null
+     * @param context      The context of the application used to setup the listener
      */
-    public void sendMessage(final @NonNull SMSMessage message, final @Nullable SMSSentListener sentListener) {
-        sendMessage(message, sentListener, null);
+    public void sendMessage(final @NonNull SMSMessage message,
+                            final @Nullable SMSSentListener sentListener,
+                            Context context) {
+        sendMessage(message, sentListener, null, context);
     }
 
     /**
@@ -107,9 +94,12 @@ public void sendMessage(final @NonNull SMSMessage message, final @Nullable SMSSe
      *
      * @param message           to be sent in the channel to a peer
      * @param deliveredListener called on message delivered or on error, can be null
+     * @param context           The context of the application used to setup the listener
      */
-    public void sendMessage(final @NonNull SMSMessage message, final @Nullable SMSDeliveredListener deliveredListener) {
-        sendMessage(message, null, deliveredListener);
+    public void sendMessage(final @NonNull SMSMessage message,
+                            final @Nullable SMSDeliveredListener deliveredListener,
+                            Context context) {
+        sendMessage(message, null, deliveredListener, context);
     }
 
     /**
@@ -119,13 +109,14 @@ public void sendMessage(final @NonNull SMSMessage message, final @Nullable SMSDe
      * @param message           to be sent in the channel to a peer
      * @param sentListener      called on message sent or on error, can be null
      * @param deliveredListener called on message delivered or on error, can be null
+     * @param context           The context of the application used to setup the listener
      */
     public void sendMessage(final @NonNull SMSMessage message,
                             final @Nullable SMSSentListener sentListener,
-                            final @Nullable SMSDeliveredListener deliveredListener) {
-        checkSetup();
-        PendingIntent sentPI = setupNewSentReceiver(message, sentListener);
-        PendingIntent deliveredPI = setupNewDeliverReceiver(message, deliveredListener);
+                            final @Nullable SMSDeliveredListener deliveredListener,
+                            Context context) {
+        PendingIntent sentPI = setupNewSentReceiver(message, sentListener, context);
+        PendingIntent deliveredPI = setupNewDeliverReceiver(message, deliveredListener, context);
         SMSCore.sendMessage(getSMSContent(message), message.getPeer().getAddress(), sentPI, deliveredPI);
     }
 
@@ -135,16 +126,19 @@ public void sendMessage(final @NonNull SMSMessage message,
      *
      * @param message  that will be sent
      * @param listener to call on broadcast received
+     * @param context  The context of the application used to setup the listener
      * @return a {@link PendingIntent} to be passed to SMSCore
      */
-    private PendingIntent setupNewSentReceiver(final @NonNull SMSMessage message, final @Nullable SMSSentListener listener) {
-        if (listener == null)
-            return null; //Doesn't make any sense to have a BroadcastReceiver if there is no listener
+    private PendingIntent setupNewSentReceiver(final @NonNull SMSMessage message,
+                                               final @Nullable SMSSentListener listener,
+                                               Context context) {
+        if (listener == null || context == null)
+            return null; //Doesn't make any sense to have a BroadcastReceiver if there is no listener or context
 
         SMSSentBroadcastReceiver onSentReceiver = new SMSSentBroadcastReceiver(message, listener);
         String actionName = SENT_MESSAGE_INTENT_ACTION + (messageCounter++);
-        context.get().registerReceiver(onSentReceiver, new IntentFilter(actionName));
-        return PendingIntent.getBroadcast(context.get(), 0, new Intent(actionName), 0);
+        context.registerReceiver(onSentReceiver, new IntentFilter(actionName));
+        return PendingIntent.getBroadcast(context, 0, new Intent(actionName), 0);
     }
 
     /**
@@ -153,26 +147,19 @@ private PendingIntent setupNewSentReceiver(final @NonNull SMSMessage message, fi
      *
      * @param message  that will be sent
      * @param listener to call on broadcast received
+     * @param context  The context of the application used to setup the listener
      * @return a {@link PendingIntent} to be passed to SMSCore
      */
-    private PendingIntent setupNewDeliverReceiver(final @NonNull SMSMessage message, final @Nullable SMSDeliveredListener listener) {
-        if (listener == null)
+    private PendingIntent setupNewDeliverReceiver(final @NonNull SMSMessage message,
+                                                  final @Nullable SMSDeliveredListener listener,
+                                                  Context context) {
+        if (listener == null || context == null)
             return null; //Doesn't make any sense to have a BroadcastReceiver if there is no listener
 
         SMSDeliveredBroadcastReceiver onDeliveredReceiver = new SMSDeliveredBroadcastReceiver(message, listener);
         String actionName = DELIVERED_MESSAGE_INTENT_ACTION + (messageCounter++);
-        context.get().registerReceiver(onDeliveredReceiver, new IntentFilter(actionName));
-        return PendingIntent.getBroadcast(context.get(), 0, new Intent(actionName), 0);
-    }
-
-    /**
-     * Checks if the handler has been setup
-     *
-     * @throws IllegalStateException if the handler has not been setup
-     */
-    private void checkSetup() {
-        if (context == null)
-            throw new IllegalStateException("You must call setup() first");
+        context.registerReceiver(onDeliveredReceiver, new IntentFilter(actionName));
+        return PendingIntent.getBroadcast(context, 0, new Intent(actionName), 0);
     }
 
     /**
@@ -181,18 +168,19 @@ private void checkSetup() {
      *
      * @param receivedListenerClassName the listener called on message received
      * @param <T>                       the class type that extends {@link SMSReceivedServiceListener} to be called
+     * @param context                   the context used to set the listener
      */
-    public <T extends SMSReceivedServiceListener> void setReceivedListener(Class<T> receivedListenerClassName) {
-        checkSetup();
-        PreferencesManager.setString(context.get(), SMSReceivedBroadcastReceiver.SERVICE_CLASS_PREFERENCES_KEY, receivedListenerClassName.toString());
+    public <T extends SMSReceivedServiceListener> void setReceivedListener(Class<T> receivedListenerClassName, Context context) {
+        PreferencesManager.setString(context, SMSReceivedBroadcastReceiver.SERVICE_CLASS_PREFERENCES_KEY, receivedListenerClassName.toString());
     }
 
     /**
      * Unsubscribe the current {@link SMSReceivedServiceListener} from being called on message arrival
+     *
+     * @param context The context used to remove the listener
      */
-    public void removeReceivedListener() {
-        checkSetup();
-        PreferencesManager.removeValue(context.get(), SMSReceivedBroadcastReceiver.SERVICE_CLASS_PREFERENCES_KEY);
+    public void removeReceivedListener(Context context) {
+        PreferencesManager.removeValue(context, SMSReceivedBroadcastReceiver.SERVICE_CLASS_PREFERENCES_KEY);
     }
 
     /**

smslibrary/src/test/java/com/eis/smslibrary/SMSHandlerTest.java

@@ -0,0 +1,46 @@
+package com.eis.smslibrary;
+
+import android.telephony.SmsManager;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Class used to test SMSManager using Mocks
+ *
+ * @author Marco Cognolato
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class SMSHandlerTest {
+
+    private SmsManager managerMock = mock(SmsManager.class);
+
+    private final String PEER_TEXT = "+393423541601";
+    private final String MESSAGE_TEXT = "valid text";
+    private final String SENT_TEXT = (char) 0x02 + MESSAGE_TEXT;
+    private final SMSPeer VALID_PEER = new SMSPeer(PEER_TEXT);
+    private final SMSMessage VALID_MESSAGE = new SMSMessage(VALID_PEER, MESSAGE_TEXT);
+    private SMSManager instance = SMSManager.getInstance();
+
+    @Test
+    public void singletonInstance() {
+        assertEquals(SMSManager.getInstance(), SMSManager.getInstance());
+    }
+
+    @Test()
+    public void validMessage_isSent() {
+        SMSCore.setManager(managerMock);
+        instance.sendMessage(VALID_MESSAGE);
+        verify(managerMock).sendTextMessage(PEER_TEXT, null, SENT_TEXT, null, null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void nullMessage_throws() {
+        instance.sendMessage(null);
+    }
+}