From abde87bf413d520b28bc6322799a193c27de0717 Mon Sep 17 00:00:00 2001
From: Juan Leyva <juanleyvadelgado@gmail.com>
Date: Thu, 26 Feb 2015 12:07:08 +0100
Subject: [PATCH] MDL-48443 webservices: Prevent duplicated user devices

---
 user/externallib.php            | 40 ++++++++++++++++++++++-----------
 user/tests/externallib_test.php | 23 +++++++++++++++++++
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/user/externallib.php b/user/externallib.php
index 7b298e81877..a07c158dee3 100644
--- a/user/externallib.php
+++ b/user/externallib.php
@@ -1119,20 +1119,34 @@ class core_user_external extends external_api {
             return $warnings;
         }
 
-        $userdevice = new stdclass;
-        $userdevice->userid     = $USER->id;
-        $userdevice->appid      = $params['appid'];
-        $userdevice->name       = $params['name'];
-        $userdevice->model      = $params['model'];
-        $userdevice->platform   = $params['platform'];
-        $userdevice->version    = $params['version'];
-        $userdevice->pushid     = $params['pushid'];
-        $userdevice->uuid       = $params['uuid'];
-        $userdevice->timecreated  = time();
-        $userdevice->timemodified = $userdevice->timecreated;
+        // Notice that we can have multiple devices because previously it was allowed to have repeated ones.
+        // Since we don't have a clear way to decide which one is the more appropiate, we update all.
+        if ($userdevices = $DB->get_records('user_devices', array('uuid' => $params['uuid'],
+                'appid' => $params['appid'], 'userid' => $USER->id))) {
 
-        if (!$DB->insert_record('user_devices', $userdevice)) {
-            throw new moodle_exception("There was a problem saving in the database the device with key: " . $params['pushid']);
+            foreach ($userdevices as $userdevice) {
+                $userdevice->version    = $params['version'];   // Maybe the user upgraded the device.
+                $userdevice->pushid     = $params['pushid'];
+                $userdevice->timemodified  = time();
+                $DB->update_record('user_devices', $userdevice);
+            }
+
+        } else {
+            $userdevice = new stdclass;
+            $userdevice->userid     = $USER->id;
+            $userdevice->appid      = $params['appid'];
+            $userdevice->name       = $params['name'];
+            $userdevice->model      = $params['model'];
+            $userdevice->platform   = $params['platform'];
+            $userdevice->version    = $params['version'];
+            $userdevice->pushid     = $params['pushid'];
+            $userdevice->uuid       = $params['uuid'];
+            $userdevice->timecreated  = time();
+            $userdevice->timemodified = $userdevice->timecreated;
+
+            if (!$DB->insert_record('user_devices', $userdevice)) {
+                throw new moodle_exception("There was a problem saving in the database the device with key: " . $params['pushid']);
+            }
         }
 
         return $warnings;
diff --git a/user/tests/externallib_test.php b/user/tests/externallib_test.php
index 3452a5be5c1..6913a007d27 100644
--- a/user/tests/externallib_test.php
+++ b/user/tests/externallib_test.php
@@ -749,6 +749,29 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
         $created = (array) $created;
 
         $this->assertEquals($device, array_intersect_key((array)$created, $device));
+
+        // Test reuse the same pushid value.
+        $warnings = core_user_external::add_user_device($device['appid'], $device['name'], $device['model'], $device['platform'],
+                                                        $device['version'], $device['pushid'], $device['uuid']);
+        // We need to execute the return values cleaning process to simulate the web service server.
+        $warnings = external_api::clean_returnvalue(core_user_external::add_user_device_returns(), $warnings);
+        $this->assertCount(1, $warnings);
+
+        // Test update and existing device.
+        $device['pushid'] = 'different than before';
+        $warnings = core_user_external::add_user_device($device['appid'], $device['name'], $device['model'], $device['platform'],
+                                                        $device['version'], $device['pushid'], $device['uuid']);
+
+        $this->assertEquals(1, $DB->count_records('user_devices'));
+        $updated = $DB->get_record('user_devices', array('pushid' => $device['pushid']));
+        $this->assertEquals($device, array_intersect_key((array)$updated, $device));
+
+        // Test creating a new device just changing the uuid.
+        $device['uuid'] = 'newuidforthesameuser';
+        $device['pushid'] = 'new different than before';
+        $warnings = core_user_external::add_user_device($device['appid'], $device['name'], $device['model'], $device['platform'],
+                                                        $device['version'], $device['pushid'], $device['uuid']);
+        $this->assertEquals(2, $DB->count_records('user_devices'));
     }
 
 }