!2246 Fix duplicate account id

Merge pull request !2246 from yujann/dupAccId

frameworks/osaccount/native/test/moduletest/os_account_manager_module_test.cpp

@@ -335,13 +335,38 @@ public:
     MOCK_METHOD2(OnAccountsSwitch, void(const int &newId, const int &oldId));
 };
 
+#ifdef ENABLE_MULTIPLE_OS_ACCOUNTS
+/**
+ * @tc.name: CreateOsAccountWithFullInfo001
+ * @tc.desc: Test next id.
+ * @tc.type: FUNC
+ * @tc.require:
+ */
+HWTEST_F(OsAccountManagerModuleTest, CreateOsAccountWithFullInfo001, TestSize.Level1)
+{
+    OsAccountInfo osAccountInfo;
+    osAccountInfo.SetLocalName("testNextID_001");
+    int expectUid = 1000; // test random uid, next account should start from 1000
+    osAccountInfo.SetLocalId(expectUid);
+    osAccountInfo.SetSerialNumber(2023023100000033); // test random input
+    osAccountInfo.SetCreateTime(1695883215000); // test random input
+    osAccountInfo.SetLastLoginTime(1695863215000); // test random input
+    EXPECT_EQ(ERR_OK, OsAccountManager::CreateOsAccountWithFullInfo(osAccountInfo));
+    EXPECT_EQ(osAccountInfo.GetLocalId(), expectUid);
+    OsAccountManager::RemoveOsAccount(osAccountInfo.GetLocalId());
+    
+    OsAccountInfo osAccountInfoOne;
+    ASSERT_EQ(OsAccountManager::CreateOsAccount("testNextID_002", OsAccountType::GUEST, osAccountInfoOne), ERR_OK);
+    EXPECT_EQ(osAccountInfoOne.GetLocalId(), expectUid + 1);
+    OsAccountManager::RemoveOsAccount(osAccountInfoOne.GetLocalId());
+}
+
 /**
  * @tc.name: OsAccountManagerModuleTest001
  * @tc.desc: Test create guest account.
  * @tc.type: FUNC
  * @tc.require: issueI4IU74
  */
-#ifdef ENABLE_MULTIPLE_OS_ACCOUNTS
 HWTEST_F(OsAccountManagerModuleTest, OsAccountManagerModuleTest001, TestSize.Level0)
 {
     OsAccountInfo osAccountInfoOne;

interfaces/innerkits/osaccount/native/include/os_account_constants.h

@@ -110,6 +110,7 @@ const char ALL_SPECIFIC_CONSTRAINTS[] = "allSpecificConstraints";
 const char USER_CONSTRAINTS_TEMPLATE[] = "UserConstraintsTemplate";
 const char TYPE_LIST[] = "TypeList";
 const char ACCOUNT_LIST[] = "AccountList";
+const char NEXT_LOCAL_ID[] = "NextLocalId";
 const char COUNT_ACCOUNT_NUM[] = "CountAccountNum";
 const char MAX_ALLOW_CREATE_ACCOUNT_ID[] = "MaxAllowCreateAccountID";
 const char SERIAL_NUMBER_NUM[] = "SerialNumber";

services/accountmgr/include/osaccount/ios_account_control.h

@@ -75,6 +75,7 @@ public:
     virtual ErrCode SetDefaultActivatedOsAccount(const int32_t id) = 0;
     virtual ErrCode GetDefaultActivatedOsAccount(int32_t &id) = 0;
     virtual ErrCode UpdateAccountIndex(const OsAccountInfo &osAccountInfo, const bool isDelete) = 0;
+    virtual ErrCode SetNextLocalId(const int32_t &nextLocalId) = 0;
 };
 }  // namespace AccountSA
 }  // namespace OHOS

services/accountmgr/include/osaccount/os_account_control_file_manager.h

@@ -91,11 +91,12 @@ public:
         const std::string &userInfoPath, const std::string &accountInfoStr, std::string &digestStr);
     ErrCode DeleteAccountInfoDigest(const std::string &userInfoPath);
     ErrCode UpdateAccountIndex(const OsAccountInfo &osAccountInfo, const bool isDelete) override;
+    ErrCode SetNextLocalId(const int32_t &nextLocalId) override;
 
 private:
     ErrCode GetDefaultOsAccountConfig(OsAccountConfig &config);
     ErrCode RemoveAccountIndex(const int32_t id);
-    int GetNextLocalId(const std::vector<std::string> &accountIdList);
+    int32_t GetNextLocalId(const std::vector<std::string> &accountIdList, int32_t startId);
     ErrCode UpdateAccountList(const std::string &idStr, bool isAdd);
     ErrCode GetAccountListFromFile(Json& accountListJson);
     ErrCode GetAccountIndexInfo(std::string &accountIndexInfo);
@@ -108,7 +109,6 @@ private:
     void BuildAndSaveBaseOAConstraintsJsonFile();
     void BuildAndSaveGlobalOAConstraintsJsonFile();
     void BuildAndSaveSpecificOAConstraintsJsonFile();
-    void GetNextLocalId(int &id, const std::vector<std::string> &accountIdList, const int nextLocalId);
     void GlobalConstraintsDataOperate(const std::string& idStr,
         const std::vector<std::string>& ConstraintStr, bool isAdd, Json &globalOAConstraintsJson);
     void SpecificConstraintsDataOperate(const std::string& idStr, const std::string& targetIdStr,
@@ -137,7 +137,6 @@ private:
 #ifdef HAS_KV_STORE_PART
     std::shared_ptr<OsAccountDatabaseOperator> osAccountDataBaseOperator_;
 #endif
-    std::int32_t nextLocalId_ = Constants::START_USER_ID;
     AccountFileWatcherMgr &accountFileWatcherMgr_;
     std::shared_ptr<OsAccountFileOperator> osAccountFileOperator_;
     std::shared_ptr<OsAccountPhotoOperator> osAccountPhotoOperator_;

services/accountmgr/src/osaccount/inner_os_account_manager.cpp

@@ -347,14 +347,19 @@ ErrCode IInnerOsAccountManager::PrepareOsAccountInfoWithFullInfo(OsAccountInfo &
     int64_t serialNumber;
     ErrCode errCode = osAccountControl_->GetSerialNumber(serialNumber);
     if (errCode != ERR_OK) {
-        ACCOUNT_LOGE("failed to GetSerialNumber, errCode %{public}d.", errCode);
+        ACCOUNT_LOGE("Failed to GetSerialNumber, errCode %{public}d.", errCode);
         return errCode;
     }
     osAccountInfo.SetSerialNumber(serialNumber);
     osAccountInfo.SetIsDataRemovable(false);
+    errCode = osAccountControl_->SetNextLocalId(osAccountInfo.GetLocalId() + 1);
+    if (errCode != ERR_OK) {
+        ACCOUNT_LOGE("Failed to SetNextLocalId, errCode %{public}d.", errCode);
+        return errCode;
+    }
     errCode = osAccountControl_->InsertOsAccount(osAccountInfo);
     if ((errCode != ERR_OK) && (errCode != ERR_OSACCOUNT_SERVICE_CONTROL_INSERT_FILE_EXISTS_ERROR)) {
-        ACCOUNT_LOGE("insert os account info err, errCode %{public}d.", errCode);
+        ACCOUNT_LOGE("Insert os account info err, errCode %{public}d.", errCode);
         return errCode;
     }
 
@@ -365,7 +370,7 @@ ErrCode IInnerOsAccountManager::PrepareOsAccountInfoWithFullInfo(OsAccountInfo &
     OsAccountType type = static_cast<OsAccountType>(osAccountInfo.GetType());
     errCode = osAccountControl_->GetConstraintsByType(type, constraints);
     if (errCode != ERR_OK) {
-        ACCOUNT_LOGE("failed to GetConstraintsByType, errCode %{public}d.", errCode);
+        ACCOUNT_LOGE("Failed to GetConstraintsByType, errCode %{public}d.", errCode);
         return errCode;
     }
     std::vector<std::string> constraintsExists = osAccountInfo.GetConstraints();

services/accountmgr/src/osaccount/os_account_control_file_manager.cpp

@@ -274,13 +274,6 @@ void OsAccountControlFileManager::Init()
     OHOS::AccountSA::GetDataByType<std::vector<std::string>>(
         accountListJson, jsonEnd, Constants::ACCOUNT_LIST, accountIdList, OHOS::AccountSA::JsonType::ARRAY);
     if (!accountIdList.empty()) {
-        std::lock_guard<std::mutex> lock(operatingIdMutex_);
-        int32_t id = 0;
-        if (!StrToInt(accountIdList[accountIdList.size() - 1], id)) {
-            ACCOUNT_LOGE("Convert localId failed");
-            return;
-        }
-        nextLocalId_ = id + 1;
         InitFileWatcherInfo(accountIdList);
     }
     ACCOUNT_LOGI("OsAccountControlFileManager Init end");
@@ -343,6 +336,7 @@ void OsAccountControlFileManager::BuildAndSaveAccountListJsonFile(const std::vec
         {Constants::MAX_ALLOW_CREATE_ACCOUNT_ID, Constants::MAX_USER_ID},
         {Constants::SERIAL_NUMBER_NUM, Constants::SERIAL_NUMBER_NUM_START},
         {Constants::IS_SERIAL_NUMBER_FULL, Constants::IS_SERIAL_NUMBER_FULL_INIT_VALUE},
+        {Constants::NEXT_LOCAL_ID, Constants::START_USER_ID + 1},
     };
     SaveAccountListToFile(accountList);
 }
@@ -936,6 +930,30 @@ ErrCode OsAccountControlFileManager::UpdateAccountIndex(const OsAccountInfo &osA
     return ERR_OK;
 }
 
+ErrCode OsAccountControlFileManager::SetNextLocalId(const int32_t &nextLocalId)
+{
+    std::lock_guard<std::mutex> lock(operatingIdMutex_);
+    Json accountListJson;
+    ErrCode result = GetAccountListFromFile(accountListJson);
+    if (result != ERR_OK) {
+        ACCOUNT_LOGE("SetNextLocalId get accountList error.");
+        return result;
+    }
+    int32_t nextLocalIdJson = -1;
+    auto jsonEnd = accountListJson.end();
+    if (!GetDataByType<std::int32_t>(accountListJson, jsonEnd,
+        Constants::NEXT_LOCAL_ID, nextLocalIdJson, JsonType::NUMBER)) {
+        ACCOUNT_LOGW("SetNextLocalId get next localId failed");
+        nextLocalIdJson = Constants::START_USER_ID + 1;
+    }
+    accountListJson[Constants::NEXT_LOCAL_ID] = std::max(nextLocalId, nextLocalIdJson);
+    result = SaveAccountListToFileAndDataBase(accountListJson);
+    if (result != ERR_OK) {
+        ACCOUNT_LOGE("SetNextLocalId save accountListJson error.");
+    }
+    return result;
+}
+
 ErrCode OsAccountControlFileManager::RemoveAccountIndex(const int32_t id)
 {
     Json accountIndexJson;
@@ -1116,39 +1134,52 @@ ErrCode OsAccountControlFileManager::GetSerialNumber(int64_t &serialNumber)
     return ERR_OK;
 }
 
-int OsAccountControlFileManager::GetNextLocalId(const std::vector<std::string> &accountIdList)
+int32_t OsAccountControlFileManager::GetNextLocalId(const std::vector<std::string> &accountIdList, int32_t startId)
 {
     do {
-        if (nextLocalId_ > Constants::MAX_USER_ID) {
+        if ((startId <= Constants::START_USER_ID) || (startId > Constants::MAX_USER_ID)) {
-            nextLocalId_ = Constants::START_USER_ID;
+            startId = Constants::START_USER_ID + 1;
         }
-        if (std::find(accountIdList.begin(), accountIdList.end(), std::to_string(nextLocalId_)) ==
+        if (std::find(accountIdList.begin(), accountIdList.end(), std::to_string(startId)) ==
             accountIdList.end()) {
             break;
         }
-        nextLocalId_++;
+        ++startId;
     } while (true);
-    return nextLocalId_;
+    return startId;
 }
 
 ErrCode OsAccountControlFileManager::GetAllowCreateId(int &id)
 {
+    std::lock_guard<std::mutex> lock(operatingIdMutex_);
     Json accountListJson;
     ErrCode result = GetAccountListFromFile(accountListJson);
     if (result != ERR_OK) {
-        ACCOUNT_LOGE("GetAllowCreateId get accountList error");
+        ACCOUNT_LOGE("GetAllowCreateId get accountList error.");
         return result;
     }
     auto jsonEnd = accountListJson.end();
     std::vector<std::string> accountIdList;
-    OHOS::AccountSA::GetDataByType<std::vector<std::string>>(
+    int32_t nextLocalId = -1;
-        accountListJson, jsonEnd, Constants::ACCOUNT_LIST, accountIdList, OHOS::AccountSA::JsonType::ARRAY);
+    if (!GetDataByType<std::vector<std::string>>(accountListJson, jsonEnd,
-    std::lock_guard<std::mutex> lock(operatingIdMutex_);
+        Constants::ACCOUNT_LIST, accountIdList, JsonType::ARRAY)) {
-    id = GetNextLocalId(accountIdList);
+        ACCOUNT_LOGE("GetAllowCreateId get accountIdList error");
-    nextLocalId_++;
+        return ERR_ACCOUNT_COMMON_BAD_JSON_FORMAT_ERROR;
-    return ERR_OK;
+    }
-}
+    if (!GetDataByType<std::int32_t>(accountListJson, jsonEnd,
+        Constants::NEXT_LOCAL_ID, nextLocalId, JsonType::NUMBER)) {
+        ACCOUNT_LOGW("GetAllowCreateId get next localId failed");
+        nextLocalId = Constants::START_USER_ID + 1;
+    }
 
+    id = GetNextLocalId(accountIdList, nextLocalId);
+    accountListJson[Constants::NEXT_LOCAL_ID] = id + 1;
+    result = SaveAccountListToFileAndDataBase(accountListJson);
+    if (result != ERR_OK) {
+        ACCOUNT_LOGE("GetAllowCreateId save accountListJson error, errCode %{public}d.", result);
+    }
+    return result;
+}
 
 ErrCode OsAccountControlFileManager::GetAccountListFromFile(Json &accountListJson)
 {

services/accountmgr/test/unittest/os_account/mock/mock_os_account_control_file_manager.h

@@ -60,6 +60,7 @@ public:
     MOCK_METHOD1(SetDefaultActivatedOsAccount, ErrCode(const int32_t initialStartupId));
     MOCK_METHOD3(UpdateGlobalOAConstraints, ErrCode(const std::string& idStr,
         const std::vector<std::string>& ConstraintStr, bool isAdd));
+    MOCK_METHOD1(SetNextLocalId, ErrCode(const int32_t &nextLocalId));
     void Init() {}
     ErrCode GetAccountIndexFromFile(Json &accountIndexJson) { return ERR_OK; }
     ErrCode IsOsAccountExists(const int id, bool &isExists) { return ERR_OK; }