Native API feedback for puller API

1. Rename registerPullAtomCallback to setPullAtomCallback
2. Rename unregisterPullAtomCallback to clearPullAtomCallback
3. Change Ns to Millis for consistency with java
4. Swap metadata and callback params in setPullAtomCallback to match
java
5. Added getters for PullAtomMetadata
6. Added libstatspull_test to test PullAtomMetadata
7. Changed the types on setAdditiveFields from int to int32_t

Test: make
Test: atest LibStatsPullTests
Test: bit libstatspull_test:*
Bug: 1507885621
Bug: 151875223
Change-Id: I5cb07bfe71b1002180403828d5e8e6a5b88ce6fe
diff --git a/lib/libstatspull/Android.bp b/lib/libstatspull/Android.bp
index 1a9cb92..0fb8f1b 100644
--- a/lib/libstatspull/Android.bp
+++ b/lib/libstatspull/Android.bp
@@ -65,3 +65,25 @@
         "//frameworks/base/apex/statsd/tests/libstatspull",
     ],
 }
+
+// Note: These unit tests only test PullAtomMetadata.
+// For full E2E tests of libstatspull, use LibStatsPullTests
+cc_test {
+    name: "libstatspull_test",
+    srcs: [
+        "tests/pull_atom_metadata_test.cpp",
+    ],
+    shared_libs: [
+        "libstatspull",
+        "libstatssocket",
+    ],
+    test_suites: ["general-tests"],
+    cflags: [
+        "-Wall",
+        "-Werror",
+        "-Wno-missing-field-initializers",
+        "-Wno-unused-variable",
+        "-Wno-unused-function",
+        "-Wno-unused-parameter",
+    ],
+}
\ No newline at end of file
diff --git a/lib/libstatspull/TEST_MAPPING b/lib/libstatspull/TEST_MAPPING
new file mode 100644
index 0000000..76f4f02
--- /dev/null
+++ b/lib/libstatspull/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+  "presubmit" : [
+    {
+      "name" : "libstatspull_test"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/lib/libstatspull/include/stats_pull_atom_callback.h b/lib/libstatspull/include/stats_pull_atom_callback.h
index 0b0df2b..0bfeec6 100644
--- a/lib/libstatspull/include/stats_pull_atom_callback.h
+++ b/lib/libstatspull/include/stats_pull_atom_callback.h
@@ -45,17 +45,27 @@
 void AStatsManager_PullAtomMetadata_release(AStatsManager_PullAtomMetadata* metadata);
 
 /**
- * Set the cool down time of the pull in nanoseconds. If two successive pulls are issued
+ * Set the cool down time of the pull in milliseconds. If two successive pulls are issued
  * within the cool down, a cached version of the first will be used for the second.
  */
-void AStatsManager_PullAtomMetadata_setCoolDownNs(AStatsManager_PullAtomMetadata* metadata,
-                                                  int64_t cool_down_ns);
+void AStatsManager_PullAtomMetadata_setCoolDownMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                      int64_t cool_down_millis);
 
 /**
- * Set the maximum time the pull can take in nanoseconds.
+ * Get the cool down time of the pull in milliseconds.
  */
-void AStatsManager_PullAtomMetadata_setTimeoutNs(AStatsManager_PullAtomMetadata* metadata,
-                                                 int64_t timeout_ns);
+int64_t AStatsManager_PullAtomMetadata_getCoolDownMillis(AStatsManager_PullAtomMetadata* metadata);
+
+/**
+ * Set the maximum time the pull can take in milliseconds.
+ */
+void AStatsManager_PullAtomMetadata_setTimeoutMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                     int64_t timeout_millis);
+
+/**
+ * Get the maximum time the pull can take in milliseconds.
+ */
+int64_t AStatsManager_PullAtomMetadata_getTimeoutMillis(AStatsManager_PullAtomMetadata* metadata);
 
 /**
  * Set the additive fields of this pulled atom.
@@ -65,7 +75,25 @@
  * will be combined when the non-additive fields are the same.
  */
 void AStatsManager_PullAtomMetadata_setAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
-                                                      int* additive_fields, int num_fields);
+                                                      int32_t* additive_fields, int32_t num_fields);
+
+/**
+ * Get the number the additive fields of this pulled atom. This is intended to be called before
+ * AStatsManager_PullAtomMetadata_getAdditiveFields to determine the size of the array.
+ */
+int32_t AStatsManager_PullAtomMetadata_getNumAdditiveFields(
+        AStatsManager_PullAtomMetadata* metadata);
+
+/**
+ * Get the additive fields of this pulled atom.
+ *
+ * \param fields an output parameter containing the additive fields for this PullAtomMetadata.
+ *               Fields is an array and it is assumed that it is at least as large as the number of
+ *               additive fields, which can be obtained by calling
+ *               AStatsManager_PullAtomMetadata_getNumAdditiveFields.
+ */
+void AStatsManager_PullAtomMetadata_getAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
+                                                      int32_t* fields);
 
 /**
  * Return codes for the result of a pull.
@@ -108,7 +136,7 @@
 typedef AStatsManager_PullAtomCallbackReturn (*AStatsManager_PullAtomCallback)(
         int32_t atom_tag, AStatsEventList* data, void* cookie);
 /**
- * Registers a callback for an atom when that atom is to be pulled. The stats service will
+ * Sets a callback for an atom when that atom is to be pulled. The stats service will
  * invoke the callback when the stats service determines that this atom needs to be
  * pulled.
  *
@@ -122,19 +150,18 @@
  * \param cookie            A pointer that will be passed back to the callback.
  *                          It has no meaning to statsd.
  */
-void AStatsManager_registerPullAtomCallback(int32_t atom_tag,
-                                            AStatsManager_PullAtomCallback callback,
-                                            AStatsManager_PullAtomMetadata* metadata, void* cookie);
+void AStatsManager_setPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata* metadata,
+                                       AStatsManager_PullAtomCallback callback, void* cookie);
 
 /**
- * Unregisters a callback for an atom when that atom is to be pulled. Note that any ongoing
+ * Clears a callback for an atom when that atom is to be pulled. Note that any ongoing
  * pulls will still occur.
  *
  * Requires the REGISTER_STATS_PULL_ATOM permission.
  *
  * \param atomTag           The tag of the atom of which to unregister
  */
-void AStatsManager_unregisterPullAtomCallback(int32_t atom_tag);
+void AStatsManager_clearPullAtomCallback(int32_t atom_tag);
 
 #ifdef __cplusplus
 }
diff --git a/lib/libstatspull/libstatspull.map.txt b/lib/libstatspull/libstatspull.map.txt
index dc3fd8b..e0e851a 100644
--- a/lib/libstatspull/libstatspull.map.txt
+++ b/lib/libstatspull/libstatspull.map.txt
@@ -2,12 +2,16 @@
     global:
         AStatsManager_PullAtomMetadata_obtain; # apex # introduced=30
         AStatsManager_PullAtomMetadata_release; # apex # introduced=30
-        AStatsManager_PullAtomMetadata_setCoolDownNs; # apex # introduced=30
-        AStatsManager_PullAtomMetadata_setTimeoutNs; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_setCoolDownMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getCoolDownMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_setTimeoutMillis; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getTimeoutMillis; # apex # introduced=30
         AStatsManager_PullAtomMetadata_setAdditiveFields; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getNumAdditiveFields; # apex # introduced=30
+        AStatsManager_PullAtomMetadata_getAdditiveFields; # apex # introduced=30
         AStatsEventList_addStatsEvent; # apex # introduced=30
-        AStatsManager_registerPullAtomCallback; # apex # introduced=30
-        AStatsManager_unregisterPullAtomCallback; # apex # introduced=30
+        AStatsManager_setPullAtomCallback; # apex # introduced=30
+        AStatsManager_clearPullAtomCallback; # apex # introduced=30
     local:
         *;
 };
diff --git a/lib/libstatspull/stats_pull_atom_callback.cpp b/lib/libstatspull/stats_pull_atom_callback.cpp
index 27e9d29..bb2351d 100644
--- a/lib/libstatspull/stats_pull_atom_callback.cpp
+++ b/lib/libstatspull/stats_pull_atom_callback.cpp
@@ -46,19 +46,19 @@
     return event;
 }
 
-static const int64_t DEFAULT_COOL_DOWN_NS = 1000000000LL;  // 1 second.
-static const int64_t DEFAULT_TIMEOUT_NS = 10000000000LL;   // 10 seconds.
+static const int64_t DEFAULT_COOL_DOWN_MILLIS = 1000LL;  // 1 second.
+static const int64_t DEFAULT_TIMEOUT_MILLIS = 10000LL;   // 10 seconds.
 
 struct AStatsManager_PullAtomMetadata {
-    int64_t cool_down_ns;
-    int64_t timeout_ns;
+    int64_t cool_down_millis;
+    int64_t timeout_millis;
     std::vector<int32_t> additive_fields;
 };
 
 AStatsManager_PullAtomMetadata* AStatsManager_PullAtomMetadata_obtain() {
     AStatsManager_PullAtomMetadata* metadata = new AStatsManager_PullAtomMetadata();
-    metadata->cool_down_ns = DEFAULT_COOL_DOWN_NS;
-    metadata->timeout_ns = DEFAULT_TIMEOUT_NS;
+    metadata->cool_down_millis = DEFAULT_COOL_DOWN_MILLIS;
+    metadata->timeout_millis = DEFAULT_TIMEOUT_MILLIS;
     metadata->additive_fields = std::vector<int32_t>();
     return metadata;
 }
@@ -67,30 +67,50 @@
     delete metadata;
 }
 
-void AStatsManager_PullAtomMetadata_setCoolDownNs(AStatsManager_PullAtomMetadata* metadata,
-                                                  int64_t cool_down_ns) {
-    metadata->cool_down_ns = cool_down_ns;
+void AStatsManager_PullAtomMetadata_setCoolDownMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                      int64_t cool_down_millis) {
+    metadata->cool_down_millis = cool_down_millis;
 }
 
-void AStatsManager_PullAtomMetadata_setTimeoutNs(AStatsManager_PullAtomMetadata* metadata,
-                                                 int64_t timeout_ns) {
-    metadata->timeout_ns = timeout_ns;
+int64_t AStatsManager_PullAtomMetadata_getCoolDownMillis(AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->cool_down_millis;
+}
+
+void AStatsManager_PullAtomMetadata_setTimeoutMillis(AStatsManager_PullAtomMetadata* metadata,
+                                                     int64_t timeout_millis) {
+    metadata->timeout_millis = timeout_millis;
+}
+
+int64_t AStatsManager_PullAtomMetadata_getTimeoutMillis(AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->timeout_millis;
 }
 
 void AStatsManager_PullAtomMetadata_setAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
-                                                      int* additive_fields, int num_fields) {
+                                                      int32_t* additive_fields,
+                                                      int32_t num_fields) {
     metadata->additive_fields.assign(additive_fields, additive_fields + num_fields);
 }
 
+int32_t AStatsManager_PullAtomMetadata_getNumAdditiveFields(
+        AStatsManager_PullAtomMetadata* metadata) {
+    return metadata->additive_fields.size();
+}
+
+void AStatsManager_PullAtomMetadata_getAdditiveFields(AStatsManager_PullAtomMetadata* metadata,
+                                                      int32_t* fields) {
+    std::copy(metadata->additive_fields.begin(), metadata->additive_fields.end(), fields);
+    return;
+}
+
 class StatsPullAtomCallbackInternal : public BnPullAtomCallback {
   public:
     StatsPullAtomCallbackInternal(const AStatsManager_PullAtomCallback callback, void* cookie,
-                                  const int64_t coolDownNs, const int64_t timeoutNs,
+                                  const int64_t coolDownMillis, const int64_t timeoutMillis,
                                   const std::vector<int32_t> additiveFields)
         : mCallback(callback),
           mCookie(cookie),
-          mCoolDownNs(coolDownNs),
-          mTimeoutNs(timeoutNs),
+          mCoolDownMillis(coolDownMillis),
+          mTimeoutMillis(timeoutMillis),
           mAdditiveFields(additiveFields) {}
 
     Status onPullAtom(int32_t atomTag,
@@ -119,15 +139,15 @@
         return Status::ok();
     }
 
-    const int64_t& getCoolDownNs() const { return mCoolDownNs; }
-    const int64_t& getTimeoutNs() const { return mTimeoutNs; }
+    int64_t getCoolDownMillis() const { return mCoolDownMillis; }
+    int64_t getTimeoutMillis() const { return mTimeoutMillis; }
     const std::vector<int32_t>& getAdditiveFields() const { return mAdditiveFields; }
 
   private:
     const AStatsManager_PullAtomCallback mCallback;
     void* mCookie;
-    const int64_t mCoolDownNs;
-    const int64_t mTimeoutNs;
+    const int64_t mCoolDownMillis;
+    const int64_t mTimeoutMillis;
     const std::vector<int32_t> mAdditiveFields;
 };
 
@@ -156,8 +176,8 @@
         pullersCopy = mPullers;
     }
     for (const auto& it : pullersCopy) {
-        statsService->registerNativePullAtomCallback(it.first, it.second->getCoolDownNs(),
-                                                     it.second->getTimeoutNs(),
+        statsService->registerNativePullAtomCallback(it.first, it.second->getCoolDownMillis(),
+                                                     it.second->getTimeoutMillis(),
                                                      it.second->getAdditiveFields(), it.second);
     }
 }
@@ -186,8 +206,8 @@
         return;
     }
 
-    statsService->registerNativePullAtomCallback(atomTag, cb->getCoolDownNs(), cb->getTimeoutNs(),
-                                                 cb->getAdditiveFields(), cb);
+    statsService->registerNativePullAtomCallback(
+            atomTag, cb->getCoolDownMillis(), cb->getTimeoutMillis(), cb->getAdditiveFields(), cb);
 }
 
 void unregisterStatsPullAtomCallbackBlocking(int32_t atomTag) {
@@ -200,12 +220,11 @@
     statsService->unregisterNativePullAtomCallback(atomTag);
 }
 
-void AStatsManager_registerPullAtomCallback(int32_t atom_tag,
-                                            AStatsManager_PullAtomCallback callback,
-                                            AStatsManager_PullAtomMetadata* metadata,
-                                            void* cookie) {
-    int64_t coolDownNs = metadata == nullptr ? DEFAULT_COOL_DOWN_NS : metadata->cool_down_ns;
-    int64_t timeoutNs = metadata == nullptr ? DEFAULT_TIMEOUT_NS : metadata->timeout_ns;
+void AStatsManager_setPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata* metadata,
+                                       AStatsManager_PullAtomCallback callback, void* cookie) {
+    int64_t coolDownMillis =
+            metadata == nullptr ? DEFAULT_COOL_DOWN_MILLIS : metadata->cool_down_millis;
+    int64_t timeoutMillis = metadata == nullptr ? DEFAULT_TIMEOUT_MILLIS : metadata->timeout_millis;
 
     std::vector<int32_t> additiveFields;
     if (metadata != nullptr) {
@@ -213,8 +232,8 @@
     }
 
     std::shared_ptr<StatsPullAtomCallbackInternal> callbackBinder =
-            SharedRefBase::make<StatsPullAtomCallbackInternal>(callback, cookie, coolDownNs,
-                                                               timeoutNs, additiveFields);
+            SharedRefBase::make<StatsPullAtomCallbackInternal>(callback, cookie, coolDownMillis,
+                                                               timeoutMillis, additiveFields);
 
     {
         std::lock_guard<std::mutex> lg(pullAtomMutex);
@@ -226,7 +245,7 @@
     registerThread.detach();
 }
 
-void AStatsManager_unregisterPullAtomCallback(int32_t atom_tag) {
+void AStatsManager_clearPullAtomCallback(int32_t atom_tag) {
     {
         std::lock_guard<std::mutex> lg(pullAtomMutex);
         // Always remove the puller from our map.
diff --git a/lib/libstatspull/tests/pull_atom_metadata_test.cpp b/lib/libstatspull/tests/pull_atom_metadata_test.cpp
new file mode 100644
index 0000000..cf19303
--- /dev/null
+++ b/lib/libstatspull/tests/pull_atom_metadata_test.cpp
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2020, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <stats_pull_atom_callback.h>
+
+namespace {
+
+static const int64_t DEFAULT_COOL_DOWN_MILLIS = 1000LL;  // 1 second.
+static const int64_t DEFAULT_TIMEOUT_MILLIS = 10000LL;   // 10 seconds.
+
+}  // anonymous namespace
+
+TEST(AStatsManager_PullAtomMetadataTest, TestEmpty) {
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetTimeoutMillis) {
+    int64_t timeoutMillis = 500;
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetCoolDownMillis) {
+    int64_t coolDownMillis = 10000;
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), 0);
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetAdditiveFields) {
+    const int numFields = 3;
+    int inputFields[numFields] = {2, 4, 6};
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setAdditiveFields(metadata, inputFields, numFields);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), DEFAULT_COOL_DOWN_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), DEFAULT_TIMEOUT_MILLIS);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), numFields);
+    int outputFields[numFields];
+    AStatsManager_PullAtomMetadata_getAdditiveFields(metadata, outputFields);
+    for (int i = 0; i < numFields; i++) {
+        EXPECT_EQ(inputFields[i], outputFields[i]);
+    }
+    AStatsManager_PullAtomMetadata_release(metadata);
+}
+
+TEST(AStatsManager_PullAtomMetadataTest, TestSetAllElements) {
+    int64_t timeoutMillis = 500;
+    int64_t coolDownMillis = 10000;
+    const int numFields = 3;
+    int inputFields[numFields] = {2, 4, 6};
+
+    AStatsManager_PullAtomMetadata* metadata = AStatsManager_PullAtomMetadata_obtain();
+    AStatsManager_PullAtomMetadata_setTimeoutMillis(metadata, timeoutMillis);
+    AStatsManager_PullAtomMetadata_setCoolDownMillis(metadata, coolDownMillis);
+    AStatsManager_PullAtomMetadata_setAdditiveFields(metadata, inputFields, numFields);
+
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getCoolDownMillis(metadata), coolDownMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getTimeoutMillis(metadata), timeoutMillis);
+    EXPECT_EQ(AStatsManager_PullAtomMetadata_getNumAdditiveFields(metadata), numFields);
+    int outputFields[numFields];
+    AStatsManager_PullAtomMetadata_getAdditiveFields(metadata, outputFields);
+    for (int i = 0; i < numFields; i++) {
+        EXPECT_EQ(inputFields[i], outputFields[i]);
+    }
+    AStatsManager_PullAtomMetadata_release(metadata);
+}