Config update for allowed log sources

Completely resets the allowed log sources from scratch.

Also:
- renamed initLogSourceWhitelist to initAllowedLogSources.
- refactored tests to have cleaner asserts on data structures

Test: atest statsd_test
Bug: 162323471
Change-Id: Ie717da732eea82c764b5b1f272080a4d97a7a8eb

3 files changed, 181 insertions, 89 deletions

diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp
index d80f9dbb4256..acc12aa884aa 100644
--- a/cmds/statsd/src/metrics/MetricsManager.cpp
+++ b/cmds/statsd/src/metrics/MetricsManager.cpp
@@ -90,61 +90,7 @@ MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config,
     mVersionStringsInReport = config.version_strings_in_metric_report();
     mInstallerInReport = config.installer_in_metric_report();
 
-    // Init allowed pushed atom uids.
-    if (config.allowed_log_source_size() == 0) {
-        mConfigValid = false;
-        ALOGE("Log source allowlist is empty! This config won't get any data. Suggest adding at "
-                      "least AID_SYSTEM and AID_STATSD to the allowed_log_source field.");
-    } else {
-        for (const auto& source : config.allowed_log_source()) {
-            auto it = UidMap::sAidToUidMapping.find(source);
-            if (it != UidMap::sAidToUidMapping.end()) {
-                mAllowedUid.push_back(it->second);
-            } else {
-                mAllowedPkg.push_back(source);
-            }
-        }
-
-        if (mAllowedUid.size() + mAllowedPkg.size() > StatsdStats::kMaxLogSourceCount) {
-            ALOGE("Too many log sources. This is likely to be an error in the config.");
-            mConfigValid = false;
-        } else {
-            initLogSourceWhiteList();
-        }
-    }
-
-    // Init default allowed pull atom uids.
-    int numPullPackages = 0;
-    for (const string& pullSource : config.default_pull_packages()) {
-        auto it = UidMap::sAidToUidMapping.find(pullSource);
-        if (it != UidMap::sAidToUidMapping.end()) {
-            numPullPackages++;
-            mDefaultPullUids.insert(it->second);
-        } else {
-            ALOGE("Default pull atom packages must be in sAidToUidMapping");
-            mConfigValid = false;
-        }
-    }
-    // Init per-atom pull atom packages.
-    for (const PullAtomPackages& pullAtomPackages : config.pull_atom_packages()) {
-        int32_t atomId = pullAtomPackages.atom_id();
-        for (const string& pullPackage : pullAtomPackages.packages()) {
-            numPullPackages++;
-            auto it = UidMap::sAidToUidMapping.find(pullPackage);
-            if (it != UidMap::sAidToUidMapping.end()) {
-                mPullAtomUids[atomId].insert(it->second);
-            } else {
-                mPullAtomPackages[atomId].insert(pullPackage);
-            }
-        }
-    }
-    if (numPullPackages > StatsdStats::kMaxPullAtomPackages) {
-        ALOGE("Too many sources in default_pull_packages and pull_atom_packages. This is likely to "
-              "be an error in the config");
-        mConfigValid = false;
-    } else {
-        initPullAtomSources();
-    }
+    createAllLogSourcesFromConfig(config);
     mPullerManager->RegisterPullUidProvider(mConfigKey, this);
 
     // Store the sub-configs used.
@@ -241,10 +187,75 @@ bool MetricsManager::updateConfig(const StatsdConfig& config, const int64_t time
     mAllAnomalyTrackers = newAnomalyTrackers;
     mAlertTrackerMap = newAlertTrackerMap;
     mAllPeriodicAlarmTrackers = newPeriodicAlarmTrackers;
+
+    mAllowedUid.clear();
+    mAllowedPkg.clear();
+    mDefaultPullUids.clear();
+    mPullAtomUids.clear();
+    mPullAtomPackages.clear();
+    createAllLogSourcesFromConfig(config);
     return mConfigValid;
 }
 
-void MetricsManager::initLogSourceWhiteList() {
+void MetricsManager::createAllLogSourcesFromConfig(const StatsdConfig& config) {
+    // Init allowed pushed atom uids.
+    if (config.allowed_log_source_size() == 0) {
+        mConfigValid = false;
+        ALOGE("Log source allowlist is empty! This config won't get any data. Suggest adding at "
+              "least AID_SYSTEM and AID_STATSD to the allowed_log_source field.");
+    } else {
+        for (const auto& source : config.allowed_log_source()) {
+            auto it = UidMap::sAidToUidMapping.find(source);
+            if (it != UidMap::sAidToUidMapping.end()) {
+                mAllowedUid.push_back(it->second);
+            } else {
+                mAllowedPkg.push_back(source);
+            }
+        }
+
+        if (mAllowedUid.size() + mAllowedPkg.size() > StatsdStats::kMaxLogSourceCount) {
+            ALOGE("Too many log sources. This is likely to be an error in the config.");
+            mConfigValid = false;
+        } else {
+            initAllowedLogSources();
+        }
+    }
+
+    // Init default allowed pull atom uids.
+    int numPullPackages = 0;
+    for (const string& pullSource : config.default_pull_packages()) {
+        auto it = UidMap::sAidToUidMapping.find(pullSource);
+        if (it != UidMap::sAidToUidMapping.end()) {
+            numPullPackages++;
+            mDefaultPullUids.insert(it->second);
+        } else {
+            ALOGE("Default pull atom packages must be in sAidToUidMapping");
+            mConfigValid = false;
+        }
+    }
+    // Init per-atom pull atom packages.
+    for (const PullAtomPackages& pullAtomPackages : config.pull_atom_packages()) {
+        int32_t atomId = pullAtomPackages.atom_id();
+        for (const string& pullPackage : pullAtomPackages.packages()) {
+            numPullPackages++;
+            auto it = UidMap::sAidToUidMapping.find(pullPackage);
+            if (it != UidMap::sAidToUidMapping.end()) {
+                mPullAtomUids[atomId].insert(it->second);
+            } else {
+                mPullAtomPackages[atomId].insert(pullPackage);
+            }
+        }
+    }
+    if (numPullPackages > StatsdStats::kMaxPullAtomPackages) {
+        ALOGE("Too many sources in default_pull_packages and pull_atom_packages. This is likely to "
+              "be an error in the config");
+        mConfigValid = false;
+    } else {
+        initPullAtomSources();
+    }
+}
+
+void MetricsManager::initAllowedLogSources() {
     std::lock_guard<std::mutex> lock(mAllowedLogSourcesMutex);
     mAllowedLogSources.clear();
     mAllowedLogSources.insert(mAllowedUid.begin(), mAllowedUid.end());
@@ -288,7 +299,7 @@ void MetricsManager::notifyAppUpgrade(const int64_t& eventTimeNs, const string&
     if (std::find(mAllowedPkg.begin(), mAllowedPkg.end(), apk) != mAllowedPkg.end()) {
         // We will re-initialize the whole list because we don't want to keep the multi mapping of
         // UID<->pkg inside MetricsManager to reduce the memory usage.
-        initLogSourceWhiteList();
+        initAllowedLogSources();
     }
 
     for (const auto& it : mPullAtomPackages) {
@@ -309,7 +320,7 @@ void MetricsManager::notifyAppRemoved(const int64_t& eventTimeNs, const string&
     if (std::find(mAllowedPkg.begin(), mAllowedPkg.end(), apk) != mAllowedPkg.end()) {
         // We will re-initialize the whole list because we don't want to keep the multi mapping of
         // UID<->pkg inside MetricsManager to reduce the memory usage.
-        initLogSourceWhiteList();
+        initAllowedLogSources();
     }
 
     for (const auto& it : mPullAtomPackages) {
@@ -329,7 +340,7 @@ void MetricsManager::onUidMapReceived(const int64_t& eventTimeNs) {
     if (mAllowedPkg.size() == 0) {
         return;
     }
-    initLogSourceWhiteList();
+    initAllowedLogSources();
 }
 
 void MetricsManager::onStatsdInitCompleted(const int64_t& eventTimeNs) {
diff --git a/cmds/statsd/src/metrics/MetricsManager.h b/cmds/statsd/src/metrics/MetricsManager.h
index 27f3d51b07ce..23048ae2d40a 100644
--- a/cmds/statsd/src/metrics/MetricsManager.h
+++ b/cmds/statsd/src/metrics/MetricsManager.h
@@ -285,10 +285,14 @@ private:
 
     std::vector<int> mMetricIndexesWithActivation;
 
-    void initLogSourceWhiteList();
+    void initAllowedLogSources();
 
     void initPullAtomSources();
 
+    // Only called on config creation/update to initialize log sources from the config.
+    // Calls initAllowedLogSources and initPullAtomSources. Sets mConfigValid to false on error.
+    void createAllLogSourcesFromConfig(const StatsdConfig& config);
+
     // The metrics that don't need to be uploaded or even reported.
     std::set<int64_t> mNoReportMetricIds;
 
@@ -330,6 +334,7 @@ private:
     FRIEND_TEST(MetricActivationE2eTest, TestCountMetricWithTwoMetricsTwoDeactivations);
 
     FRIEND_TEST(MetricsManagerTest, TestLogSources);
+    FRIEND_TEST(MetricsManagerTest, TestLogSourcesOnConfigUpdate);
 
     FRIEND_TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead);
     FRIEND_TEST(StatsLogProcessorTest, TestActivationOnBoot);
diff --git a/cmds/statsd/tests/MetricsManager_test.cpp b/cmds/statsd/tests/MetricsManager_test.cpp
index 2dd774e7dfc9..f05ec490dcee 100644
--- a/cmds/statsd/tests/MetricsManager_test.cpp
+++ b/cmds/statsd/tests/MetricsManager_test.cpp
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <private/android_filesystem_config.h>
 #include <stdio.h>
@@ -92,8 +93,12 @@ StatsdConfig buildGoodConfig() {
     return config;
 }
 
-bool isSubset(const set<int32_t>& set1, const set<int32_t>& set2) {
-    return std::includes(set2.begin(), set2.end(), set1.begin(), set1.end());
+set<int32_t> unionSet(const vector<set<int32_t>> sets) {
+    set<int32_t> toRet;
+    for (const set<int32_t>& s : sets) {
+        toRet.insert(s.begin(), s.end());
+    }
+    return toRet;
 }
 }  // anonymous namespace
 
@@ -110,9 +115,7 @@ TEST(MetricsManagerTest, TestLogSources) {
     pkgToUids[app2] = app2Uids;
     pkgToUids[app3] = app3Uids;
 
-    int32_t atom1 = 10;
-    int32_t atom2 = 20;
-    int32_t atom3 = 30;
+    int32_t atom1 = 10, atom2 = 20, atom3 = 30;
     sp<MockUidMap> uidMap = new StrictMock<MockUidMap>();
     EXPECT_CALL(*uidMap, getAppUid(_))
             .Times(4)
@@ -150,42 +153,115 @@ TEST(MetricsManagerTest, TestLogSources) {
 
     MetricsManager metricsManager(kConfigKey, config, timeBaseSec, timeBaseSec, uidMap,
                                   pullerManager, anomalyAlarmMonitor, periodicAlarmMonitor);
+    EXPECT_TRUE(metricsManager.isConfigValid());
+
+    EXPECT_THAT(metricsManager.mAllowedUid, ElementsAre(AID_SYSTEM));
+    EXPECT_THAT(metricsManager.mAllowedPkg, ElementsAre(app1));
+    EXPECT_THAT(metricsManager.mAllowedLogSources,
+                ContainerEq(unionSet(vector<set<int32_t>>({app1Uids, {AID_SYSTEM}}))));
+    EXPECT_THAT(metricsManager.mDefaultPullUids, ContainerEq(defaultPullUids));
+
+    vector<int32_t> atom1Uids = metricsManager.getPullAtomUids(atom1);
+    EXPECT_THAT(atom1Uids,
+                UnorderedElementsAreArray(unionSet({defaultPullUids, app1Uids, app3Uids})));
+
+    vector<int32_t> atom2Uids = metricsManager.getPullAtomUids(atom2);
+    EXPECT_THAT(atom2Uids,
+                UnorderedElementsAreArray(unionSet({defaultPullUids, app2Uids, {AID_STATSD}})));
+
+    vector<int32_t> atom3Uids = metricsManager.getPullAtomUids(atom3);
+    EXPECT_THAT(atom3Uids, UnorderedElementsAreArray(defaultPullUids));
+}
+
+TEST(MetricsManagerTest, TestLogSourcesOnConfigUpdate) {
+    string app1 = "app1";
+    set<int32_t> app1Uids = {1111, 11111};
+    string app2 = "app2";
+    set<int32_t> app2Uids = {2222};
+    string app3 = "app3";
+    set<int32_t> app3Uids = {3333, 1111};
 
+    map<string, set<int32_t>> pkgToUids;
+    pkgToUids[app1] = app1Uids;
+    pkgToUids[app2] = app2Uids;
+    pkgToUids[app3] = app3Uids;
+
+    int32_t atom1 = 10, atom2 = 20, atom3 = 30;
+    sp<MockUidMap> uidMap = new StrictMock<MockUidMap>();
+    EXPECT_CALL(*uidMap, getAppUid(_))
+            .Times(8)
+            .WillRepeatedly(Invoke([&pkgToUids](const string& pkg) {
+                const auto& it = pkgToUids.find(pkg);
+                if (it != pkgToUids.end()) {
+                    return it->second;
+                }
+                return set<int32_t>();
+            }));
+    sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
+    EXPECT_CALL(*pullerManager, RegisterPullUidProvider(kConfigKey, _)).Times(1);
+    EXPECT_CALL(*pullerManager, UnregisterPullUidProvider(kConfigKey, _)).Times(1);
+
+    sp<AlarmMonitor> anomalyAlarmMonitor;
+    sp<AlarmMonitor> periodicAlarmMonitor;
+
+    StatsdConfig config;
+    config.add_allowed_log_source("AID_SYSTEM");
+    config.add_allowed_log_source(app1);
+    config.add_default_pull_packages("AID_SYSTEM");
+    config.add_default_pull_packages("AID_ROOT");
+
+    PullAtomPackages* pullAtomPackages = config.add_pull_atom_packages();
+    pullAtomPackages->set_atom_id(atom1);
+    pullAtomPackages->add_packages(app1);
+    pullAtomPackages->add_packages(app3);
+
+    pullAtomPackages = config.add_pull_atom_packages();
+    pullAtomPackages->set_atom_id(atom2);
+    pullAtomPackages->add_packages(app2);
+    pullAtomPackages->add_packages("AID_STATSD");
+
+    MetricsManager metricsManager(kConfigKey, config, timeBaseSec, timeBaseSec, uidMap,
+                                  pullerManager, anomalyAlarmMonitor, periodicAlarmMonitor);
     EXPECT_TRUE(metricsManager.isConfigValid());
 
-    ASSERT_EQ(metricsManager.mAllowedUid.size(), 1);
-    EXPECT_EQ(metricsManager.mAllowedUid[0], AID_SYSTEM);
+    // Update with new allowed log sources.
+    StatsdConfig newConfig;
+    newConfig.add_allowed_log_source("AID_ROOT");
+    newConfig.add_allowed_log_source(app2);
+    newConfig.add_default_pull_packages("AID_SYSTEM");
+    newConfig.add_default_pull_packages("AID_STATSD");
 
-    ASSERT_EQ(metricsManager.mAllowedPkg.size(), 1);
-    EXPECT_EQ(metricsManager.mAllowedPkg[0], app1);
+    pullAtomPackages = newConfig.add_pull_atom_packages();
+    pullAtomPackages->set_atom_id(atom2);
+    pullAtomPackages->add_packages(app1);
+    pullAtomPackages->add_packages(app3);
 
-    ASSERT_EQ(metricsManager.mAllowedLogSources.size(), 3);
-    EXPECT_TRUE(isSubset({AID_SYSTEM}, metricsManager.mAllowedLogSources));
-    EXPECT_TRUE(isSubset(app1Uids, metricsManager.mAllowedLogSources));
+    pullAtomPackages = newConfig.add_pull_atom_packages();
+    pullAtomPackages->set_atom_id(atom3);
+    pullAtomPackages->add_packages(app2);
+    pullAtomPackages->add_packages("AID_ADB");
+
+    metricsManager.updateConfig(newConfig, timeBaseSec, timeBaseSec, anomalyAlarmMonitor,
+                                periodicAlarmMonitor);
+    EXPECT_TRUE(metricsManager.isConfigValid());
 
-    ASSERT_EQ(metricsManager.mDefaultPullUids.size(), 2);
-    EXPECT_TRUE(isSubset(defaultPullUids, metricsManager.mDefaultPullUids));
-    ;
+    EXPECT_THAT(metricsManager.mAllowedUid, ElementsAre(AID_ROOT));
+    EXPECT_THAT(metricsManager.mAllowedPkg, ElementsAre(app2));
+    EXPECT_THAT(metricsManager.mAllowedLogSources,
+                ContainerEq(unionSet(vector<set<int32_t>>({app2Uids, {AID_ROOT}}))));
+    const set<int32_t> defaultPullUids = {AID_SYSTEM, AID_STATSD};
+    EXPECT_THAT(metricsManager.mDefaultPullUids, ContainerEq(defaultPullUids));
 
     vector<int32_t> atom1Uids = metricsManager.getPullAtomUids(atom1);
-    ASSERT_EQ(atom1Uids.size(), 5);
-    set<int32_t> expectedAtom1Uids;
-    expectedAtom1Uids.insert(defaultPullUids.begin(), defaultPullUids.end());
-    expectedAtom1Uids.insert(app1Uids.begin(), app1Uids.end());
-    expectedAtom1Uids.insert(app3Uids.begin(), app3Uids.end());
-    EXPECT_TRUE(isSubset(expectedAtom1Uids, set<int32_t>(atom1Uids.begin(), atom1Uids.end())));
+    EXPECT_THAT(atom1Uids, UnorderedElementsAreArray(defaultPullUids));
 
     vector<int32_t> atom2Uids = metricsManager.getPullAtomUids(atom2);
-    ASSERT_EQ(atom2Uids.size(), 4);
-    set<int32_t> expectedAtom2Uids;
-    expectedAtom1Uids.insert(defaultPullUids.begin(), defaultPullUids.end());
-    expectedAtom1Uids.insert(app2Uids.begin(), app2Uids.end());
-    expectedAtom1Uids.insert(AID_STATSD);
-    EXPECT_TRUE(isSubset(expectedAtom2Uids, set<int32_t>(atom2Uids.begin(), atom2Uids.end())));
+    EXPECT_THAT(atom2Uids,
+                UnorderedElementsAreArray(unionSet({defaultPullUids, app1Uids, app3Uids})));
 
     vector<int32_t> atom3Uids = metricsManager.getPullAtomUids(atom3);
-    ASSERT_EQ(atom3Uids.size(), 2);
-    EXPECT_TRUE(isSubset(defaultPullUids, set<int32_t>(atom3Uids.begin(), atom3Uids.end())));
+    EXPECT_THAT(atom3Uids,
+                UnorderedElementsAreArray(unionSet({defaultPullUids, app2Uids, {AID_ADB}})));
 }
 
 TEST(MetricsManagerTest, TestCheckLogCredentialsWhitelistedAtom) {