Merge pull request #17 from adam-p/add-HasInstanceID

Add "hasInstanceID" to diagnostic data
Psiphon-Inc · Nov 23, 2021 · 5d17ed6 · 5d17ed6
2 parents 613169d + 1e2ecb8
commit 5d17ed6
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 0 deletions.
diff --git a/psicash.cpp b/psicash.cpp
@@ -495,6 +495,7 @@ json PsiCash::GetDiagnosticInfo(bool lite) const {
     json j = json::object();
 
     j["test"] = test_;
+    j["hasInstanceID"] = user_data_->HasInstanceID();
     j["isLoggedOutAccount"] = user_data_->GetIsLoggedOutAccount();
     j["validTokenTypes"] = user_data_->ValidTokenTypes();
     j["isAccount"] = IsAccount();

diff --git a/psicash_test.cpp b/psicash_test.cpp
@@ -1100,6 +1100,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         ASSERT_FALSE(err);
 
         auto want = R"|({
+        "hasInstanceID":true,
         "balance":0,
         "isAccount":false,
         "isLoggedOutAccount":false,
@@ -1113,6 +1114,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         ASSERT_EQ(j, want);
 
         want = R"|({
+        "hasInstanceID":true,
         "balance":0,
         "isAccount":false,
         "isLoggedOutAccount":false,
@@ -1132,6 +1134,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         ASSERT_FALSE(err);
 
         auto want = R"|({
+        "hasInstanceID":true,
         "balance":0,
         "isAccount":false,
         "isLoggedOutAccount":false,
@@ -1145,6 +1148,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         ASSERT_EQ(j, want);
 
         want = R"|({
+        "hasInstanceID":true,
         "balance":0,
         "isAccount":false,
         "isLoggedOutAccount":false,
@@ -1163,6 +1167,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         pc.user_data().SetAuthTokens({{"a", {"a"}}, {"b", {"b"}}, {"c", {"c"}}}, true, "username");
         // pc.user_data().SetServerTimeDiff() // too hard to do reliably
         want = R"|({
+        "hasInstanceID":true,
         "balance":12345,
         "isAccount":true,
         "isLoggedOutAccount":false,
@@ -1175,6 +1180,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         j = pc.GetDiagnosticInfo(/*lite=*/false);
         ASSERT_EQ(j, want);
         want = R"|({
+        "hasInstanceID":true,
         "balance":12345,
         "isAccount":true,
         "isLoggedOutAccount":false,
@@ -1188,6 +1194,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
 
         pc.user_data().DeleteUserData(/*is_logged_out_account=*/true);
         want = R"|({
+        "hasInstanceID":true,
         "balance":0,
         "isAccount":true,
         "isLoggedOutAccount":true,
@@ -1200,6 +1207,7 @@ TEST_F(TestPsiCash, GetDiagnosticInfo) {
         j = pc.GetDiagnosticInfo(/*lite=*/false);
         ASSERT_EQ(j, want);
         want = R"|({
+        "hasInstanceID":true,
         "balance":0,
         "isAccount":true,
         "isLoggedOutAccount":true,

diff --git a/userdata.cpp b/userdata.cpp
@@ -163,6 +163,11 @@ std::string UserData::GetInstanceID() const {
     return *v;
 }
 
+bool UserData::HasInstanceID() const {
+    auto v = datastore_.Get<string>(kInstanceIDPtr);
+    return !!v && v->length() > 0;
+}
+
 bool UserData::GetIsLoggedOutAccount() const {
     auto v = datastore_.Get<bool>(kIsLoggedOutAccountPtr);
     if (!v) {

diff --git a/userdata.hpp b/userdata.hpp
@@ -92,6 +92,8 @@ class UserData {
     error::Error DeleteUserData(bool isLoggedOutAccount);
 
     std::string GetInstanceID() const;
+    /// Returns true if there is a non-empty instance ID in the datstore.
+    bool HasInstanceID() const;
 
     bool GetIsLoggedOutAccount() const;
     error::Error SetIsLoggedOutAccount(bool v);

diff --git a/userdata_test.cpp b/userdata_test.cpp
@@ -176,6 +176,46 @@ TEST_F(TestUserData, GetInstanceID)
     ASSERT_NE(v1, v2);
 }
 
+TEST_F(TestUserData, HasInstanceID)
+{
+    auto temp_dir = GetTempDir();
+
+    {
+        UserData ud;
+        auto err = ud.Init(temp_dir.c_str(), dev);
+        ASSERT_FALSE(err);
+        ASSERT_TRUE(ud.HasInstanceID());
+
+        // Clear should generate a new instance ID
+        ud.Clear();
+        ASSERT_TRUE(ud.HasInstanceID());
+    }
+
+    {
+        auto instance_id_ptr = "/instance/instanceID"_json_pointer;
+
+        Datastore ds;
+        auto err = ds.Init(temp_dir, GetSuffix(dev));
+        ASSERT_FALSE(err);
+
+        auto instance_id = ds.Get<string>(instance_id_ptr);
+        ASSERT_TRUE(instance_id);
+        ASSERT_THAT(*instance_id, Not(IsEmpty()));
+
+        // Clear the instanceID so that it's absent later
+        err = ds.Set(instance_id_ptr, nullptr);
+        ASSERT_FALSE(err);
+    }
+
+    {
+        // Now we shouldn't have an instance ID
+        UserData ud;
+        auto err = ud.Init(temp_dir.c_str(), dev);
+        ASSERT_FALSE(err);
+        ASSERT_FALSE(ud.HasInstanceID());
+    }
+}
+
 TEST_F(TestUserData, IsLoggedOutAccount)
 {
     UserData ud;