MediaMetrics: Update to use fluent style recording

Move methods into Prop class.
Use const for getters.

Test: mediametrics_tests and dumpsys media.metrics
Change-Id: I76cdcce4f966ce74c44d4db019b4ce0096e567de
diff --git a/media/libmediametrics/MediaAnalyticsItem.cpp b/media/libmediametrics/MediaAnalyticsItem.cpp
index 20b10db..a4efa49 100644
--- a/media/libmediametrics/MediaAnalyticsItem.cpp
+++ b/media/libmediametrics/MediaAnalyticsItem.cpp
@@ -45,14 +45,6 @@
 // the service is off.
 #define SVC_TRIES               2
 
-// the few universal keys we have
-const MediaAnalyticsItem::Key MediaAnalyticsItem::kKeyAny  = "any";
-const MediaAnalyticsItem::Key MediaAnalyticsItem::kKeyNone  = "none";
-
-const char * const MediaAnalyticsItem::EnabledProperty  = "media.metrics.enabled";
-const char * const MediaAnalyticsItem::EnabledPropertyPersist  = "persist.media.metrics.enabled";
-const int MediaAnalyticsItem::EnabledProperty_default  = 1;
-
 // So caller doesn't need to know size of allocated space
 MediaAnalyticsItem *MediaAnalyticsItem::create()
 {
@@ -75,30 +67,6 @@
     return handle;
 }
 
-// access functions for the class
-MediaAnalyticsItem::MediaAnalyticsItem()
-    : mPid(-1),
-      mUid(-1),
-      mPkgVersionCode(0),
-      mTimestamp(0),
-      mPropCount(0), mPropSize(0), mProps(NULL)
-{
-    mKey = MediaAnalyticsItem::kKeyNone;
-}
-
-MediaAnalyticsItem::MediaAnalyticsItem(MediaAnalyticsItem::Key key)
-    : mPid(-1),
-      mUid(-1),
-      mPkgVersionCode(0),
-      mTimestamp(0),
-      mPropCount(0), mPropSize(0), mProps(NULL)
-{
-    if (DEBUG_ALLOCATIONS) {
-        ALOGD("Allocate MediaAnalyticsItem @ %p", this);
-    }
-    mKey = key;
-}
-
 MediaAnalyticsItem::~MediaAnalyticsItem() {
     if (DEBUG_ALLOCATIONS) {
         ALOGD("Destroy  MediaAnalyticsItem @ %p", this);
@@ -114,7 +82,7 @@
     // clean attributes
     // contents of the attributes
     for (size_t i = 0 ; i < mPropCount; i++ ) {
-        clearProp(&mProps[i]);
+        mProps[i].clear();
     }
     // the attribute records themselves
     if (mProps != NULL) {
@@ -142,7 +110,7 @@
         // properties aka attributes
         dst->growProps(this->mPropCount);
         for(size_t i=0;i<mPropCount;i++) {
-            copyProp(&dst->mProps[i], &this->mProps[i]);
+            dst->mProps[i] = this->mProps[i];
         }
         dst->mPropCount = this->mPropCount;
     }
@@ -203,22 +171,16 @@
 }
 
 // find the proper entry in the list
-size_t MediaAnalyticsItem::findPropIndex(const char *name, size_t len)
+size_t MediaAnalyticsItem::findPropIndex(const char *name, size_t len) const
 {
     size_t i = 0;
     for (; i < mPropCount; i++) {
-        Prop *prop = &mProps[i];
-        if (prop->mNameLen != len) {
-            continue;
-        }
-        if (memcmp(name, prop->mName, len) == 0) {
-            break;
-        }
+        if (mProps[i].isNamed(name, len)) break;
     }
     return i;
 }
 
-MediaAnalyticsItem::Prop *MediaAnalyticsItem::findProp(const char *name) {
+MediaAnalyticsItem::Prop *MediaAnalyticsItem::findProp(const char *name) const {
     size_t len = strlen(name);
     size_t i = findPropIndex(name, len);
     if (i < mPropCount) {
@@ -227,16 +189,6 @@
     return NULL;
 }
 
-void MediaAnalyticsItem::Prop::setName(const char *name, size_t len) {
-    free((void *)mName);
-    mName = (const char *) malloc(len+1);
-    LOG_ALWAYS_FATAL_IF(mName == NULL,
-                        "failed malloc() for property '%s' (len %zu)",
-                        name, len);
-    memcpy ((void *)mName, name, len+1);
-    mNameLen = len;
-}
-
 // consider this "find-or-allocate".
 // caller validates type and uses clearPropValue() accordingly
 MediaAnalyticsItem::Prop *MediaAnalyticsItem::allocateProp(const char *name) {
@@ -266,12 +218,10 @@
     size_t len = strlen(name);
     size_t i = findPropIndex(name, len);
     if (i < mPropCount) {
-        Prop *prop = &mProps[i];
-        clearProp(prop);
+        mProps[i].clear();
         if (i != mPropCount-1) {
             // in the middle, bring last one down to fill gap
-            copyProp(prop, &mProps[mPropCount-1]);
-            clearProp(&mProps[mPropCount-1]);
+            mProps[i].swap(mProps[mPropCount-1]);
         }
         mPropCount--;
         return true;
@@ -279,206 +229,6 @@
     return false;
 }
 
-// set the values
-void MediaAnalyticsItem::setInt32(MediaAnalyticsItem::Attr name, int32_t value) {
-    Prop *prop = allocateProp(name);
-    if (prop != NULL) {
-        clearPropValue(prop);
-        prop->mType = kTypeInt32;
-        prop->u.int32Value = value;
-    }
-}
-
-void MediaAnalyticsItem::setInt64(MediaAnalyticsItem::Attr name, int64_t value) {
-    Prop *prop = allocateProp(name);
-    if (prop != NULL) {
-        clearPropValue(prop);
-        prop->mType = kTypeInt64;
-        prop->u.int64Value = value;
-    }
-}
-
-void MediaAnalyticsItem::setDouble(MediaAnalyticsItem::Attr name, double value) {
-    Prop *prop = allocateProp(name);
-    if (prop != NULL) {
-        clearPropValue(prop);
-        prop->mType = kTypeDouble;
-        prop->u.doubleValue = value;
-    }
-}
-
-void MediaAnalyticsItem::setCString(MediaAnalyticsItem::Attr name, const char *value) {
-
-    Prop *prop = allocateProp(name);
-    // any old value will be gone
-    if (prop != NULL) {
-        clearPropValue(prop);
-        prop->mType = kTypeCString;
-        prop->u.CStringValue = strdup(value);
-    }
-}
-
-void MediaAnalyticsItem::setRate(MediaAnalyticsItem::Attr name, int64_t count, int64_t duration) {
-    Prop *prop = allocateProp(name);
-    if (prop != NULL) {
-        clearPropValue(prop);
-        prop->mType = kTypeRate;
-        prop->u.rate.count = count;
-        prop->u.rate.duration = duration;
-    }
-}
-
-
-// find/add/set fused into a single operation
-void MediaAnalyticsItem::addInt32(MediaAnalyticsItem::Attr name, int32_t value) {
-    Prop *prop = allocateProp(name);
-    if (prop == NULL) {
-        return;
-    }
-    switch (prop->mType) {
-        case kTypeInt32:
-            prop->u.int32Value += value;
-            break;
-        default:
-            clearPropValue(prop);
-            prop->mType = kTypeInt32;
-            prop->u.int32Value = value;
-            break;
-    }
-}
-
-void MediaAnalyticsItem::addInt64(MediaAnalyticsItem::Attr name, int64_t value) {
-    Prop *prop = allocateProp(name);
-    if (prop == NULL) {
-        return;
-    }
-    switch (prop->mType) {
-        case kTypeInt64:
-            prop->u.int64Value += value;
-            break;
-        default:
-            clearPropValue(prop);
-            prop->mType = kTypeInt64;
-            prop->u.int64Value = value;
-            break;
-    }
-}
-
-void MediaAnalyticsItem::addRate(MediaAnalyticsItem::Attr name, int64_t count, int64_t duration) {
-    Prop *prop = allocateProp(name);
-    if (prop == NULL) {
-        return;
-    }
-    switch (prop->mType) {
-        case kTypeRate:
-            prop->u.rate.count += count;
-            prop->u.rate.duration += duration;
-            break;
-        default:
-            clearPropValue(prop);
-            prop->mType = kTypeRate;
-            prop->u.rate.count = count;
-            prop->u.rate.duration = duration;
-            break;
-    }
-}
-
-void MediaAnalyticsItem::addDouble(MediaAnalyticsItem::Attr name, double value) {
-    Prop *prop = allocateProp(name);
-    if (prop == NULL) {
-        return;
-    }
-    switch (prop->mType) {
-        case kTypeDouble:
-            prop->u.doubleValue += value;
-            break;
-        default:
-            clearPropValue(prop);
-            prop->mType = kTypeDouble;
-            prop->u.doubleValue = value;
-            break;
-    }
-}
-
-// find & extract values
-bool MediaAnalyticsItem::getInt32(MediaAnalyticsItem::Attr name, int32_t *value) {
-    Prop *prop = findProp(name);
-    if (prop == NULL || prop->mType != kTypeInt32) {
-        return false;
-    }
-    if (value != NULL) {
-        *value = prop->u.int32Value;
-    }
-    return true;
-}
-
-bool MediaAnalyticsItem::getInt64(MediaAnalyticsItem::Attr name, int64_t *value) {
-    Prop *prop = findProp(name);
-    if (prop == NULL || prop->mType != kTypeInt64) {
-        return false;
-    }
-    if (value != NULL) {
-        *value = prop->u.int64Value;
-    }
-    return true;
-}
-
-bool MediaAnalyticsItem::getRate(MediaAnalyticsItem::Attr name, int64_t *count, int64_t *duration, double *rate) {
-    Prop *prop = findProp(name);
-    if (prop == NULL || prop->mType != kTypeRate) {
-        return false;
-    }
-    if (count != NULL) {
-        *count = prop->u.rate.count;
-    }
-    if (duration != NULL) {
-        *duration = prop->u.rate.duration;
-    }
-    if (rate != NULL) {
-        double r = 0.0;
-        if (prop->u.rate.duration != 0) {
-            r = prop->u.rate.count / (double) prop->u.rate.duration;
-        }
-        *rate = r;
-    }
-    return true;
-}
-
-bool MediaAnalyticsItem::getDouble(MediaAnalyticsItem::Attr name, double *value) {
-    Prop *prop = findProp(name);
-    if (prop == NULL || prop->mType != kTypeDouble) {
-        return false;
-    }
-    if (value != NULL) {
-        *value = prop->u.doubleValue;
-    }
-    return true;
-}
-
-// caller responsible for the returned string
-bool MediaAnalyticsItem::getCString(MediaAnalyticsItem::Attr name, char **value) {
-    Prop *prop = findProp(name);
-    if (prop == NULL || prop->mType != kTypeCString) {
-        return false;
-    }
-    if (value != NULL) {
-        *value = strdup(prop->u.CStringValue);
-    }
-    return true;
-}
-
-bool MediaAnalyticsItem::getString(MediaAnalyticsItem::Attr name, std::string *value) {
-    Prop *prop = findProp(name);
-    if (prop == NULL || prop->mType != kTypeCString) {
-        return false;
-    }
-    if (value != NULL) {
-        // std::string makes a copy for us
-        *value = prop->u.CStringValue;
-    }
-    return true;
-}
-
 // remove indicated keys and their values
 // return value is # keys removed
 int32_t MediaAnalyticsItem::filter(int n, MediaAnalyticsItem::Attr attrs[]) {
@@ -496,12 +246,12 @@
         } else if (j+1 == mPropCount) {
             // last one, shorten
             zapped++;
-            clearProp(&mProps[j]);
+            mProps[j].clear();
             mPropCount--;
         } else {
             // in the middle, bring last one down and shorten
             zapped++;
-            clearProp(&mProps[j]);
+            mProps[j].clear();
             mProps[j] = mProps[mPropCount-1];
             mPropCount--;
         }
@@ -519,13 +269,13 @@
     for (ssize_t i = mPropCount-1 ; i >=0 ;  i--) {
         Prop *prop = &mProps[i];
         for (ssize_t j = 0; j < n ; j++) {
-            if (strcmp(prop->mName, attrs[j]) == 0) {
-                clearProp(prop);
+            if (prop->isNamed(attrs[j])) {
+                prop->clear();
                 zapped++;
                 if (i != (ssize_t)(mPropCount-1)) {
                     *prop = mProps[mPropCount-1];
                 }
-                initProp(&mProps[mPropCount-1]);
+                mProps[mPropCount-1].clear();
                 mPropCount--;
                 break;
             }
@@ -540,63 +290,6 @@
     return filter(1, &name);
 }
 
-// handle individual items/properties stored within the class
-//
-
-void MediaAnalyticsItem::initProp(Prop *prop) {
-    if (prop != NULL) {
-        prop->mName = NULL;
-        prop->mNameLen = 0;
-
-        prop->mType = kTypeNone;
-    }
-}
-
-void MediaAnalyticsItem::clearProp(Prop *prop)
-{
-    if (prop != NULL) {
-        if (prop->mName != NULL) {
-            free((void *)prop->mName);
-            prop->mName = NULL;
-            prop->mNameLen = 0;
-        }
-
-        clearPropValue(prop);
-    }
-}
-
-void MediaAnalyticsItem::clearPropValue(Prop *prop)
-{
-    if (prop != NULL) {
-        if (prop->mType == kTypeCString && prop->u.CStringValue != NULL) {
-            free(prop->u.CStringValue);
-            prop->u.CStringValue = NULL;
-        }
-        prop->mType = kTypeNone;
-    }
-}
-
-void MediaAnalyticsItem::copyProp(Prop *dst, const Prop *src)
-{
-    // get rid of any pointers in the dst
-    clearProp(dst);
-
-    *dst = *src;
-
-    // fix any pointers that we blindly copied, so we have our own copies
-    if (dst->mName) {
-        void *p =  malloc(dst->mNameLen + 1);
-        LOG_ALWAYS_FATAL_IF(p == NULL,
-                            "failed malloc() duping property '%s' (len %zu)",
-                            dst->mName, dst->mNameLen);
-        memcpy (p, src->mName, dst->mNameLen + 1);
-        dst->mName = (const char *) p;
-    }
-    if (dst->mType == kTypeCString) {
-        dst->u.CStringValue = strdup(src->u.CStringValue);
-    }
-}
-
 bool MediaAnalyticsItem::growProps(int increment)
 {
     if (increment <= 0) {
@@ -607,7 +300,7 @@
 
     if (ni != NULL) {
         for (int i = mPropSize; i < nsize; i++) {
-            initProp(&ni[i]);
+            new (&ni[i]) Prop(); // placement new
         }
         mProps = ni;
         mPropSize = nsize;
@@ -707,36 +400,11 @@
     data->writeInt64(mTimestamp);
 
     // set of items
-    int count = mPropCount;
+    const size_t count = mPropCount;
     data->writeInt32(count);
-    for (int i = 0 ; i < count; i++ ) {
-            Prop *prop = &mProps[i];
-            data->writeCString(prop->mName);
-            data->writeInt32(prop->mType);
-            switch (prop->mType) {
-                case MediaAnalyticsItem::kTypeInt32:
-                        data->writeInt32(prop->u.int32Value);
-                        break;
-                case MediaAnalyticsItem::kTypeInt64:
-                        data->writeInt64(prop->u.int64Value);
-                        break;
-                case MediaAnalyticsItem::kTypeDouble:
-                        data->writeDouble(prop->u.doubleValue);
-                        break;
-                case MediaAnalyticsItem::kTypeRate:
-                        data->writeInt64(prop->u.rate.count);
-                        data->writeInt64(prop->u.rate.duration);
-                        break;
-                case MediaAnalyticsItem::kTypeCString:
-                        data->writeCString(prop->u.CStringValue);
-                        break;
-                default:
-                        ALOGE("found bad Prop type: %d, idx %d, name %s",
-                              prop->mType, i, prop->mName);
-                        break;
-            }
+    for (size_t i = 0 ; i < count; i++ ) {
+        mProps[i].writeToParcel(data);
     }
-
     return 0;
 }
 
@@ -808,39 +476,8 @@
     snprintf(buffer, sizeof(buffer), "%d:", count);
     result.append(buffer);
     for (int i = 0 ; i < count; i++ ) {
-            Prop *prop = &mProps[i];
-            switch (prop->mType) {
-                case MediaAnalyticsItem::kTypeInt32:
-                        snprintf(buffer,sizeof(buffer),
-                        "%s=%d:", prop->mName, prop->u.int32Value);
-                        break;
-                case MediaAnalyticsItem::kTypeInt64:
-                        snprintf(buffer,sizeof(buffer),
-                        "%s=%" PRId64 ":", prop->mName, prop->u.int64Value);
-                        break;
-                case MediaAnalyticsItem::kTypeDouble:
-                        snprintf(buffer,sizeof(buffer),
-                        "%s=%e:", prop->mName, prop->u.doubleValue);
-                        break;
-                case MediaAnalyticsItem::kTypeRate:
-                        snprintf(buffer,sizeof(buffer),
-                        "%s=%" PRId64 "/%" PRId64 ":", prop->mName,
-                        prop->u.rate.count, prop->u.rate.duration);
-                        break;
-                case MediaAnalyticsItem::kTypeCString:
-                        snprintf(buffer,sizeof(buffer), "%s=", prop->mName);
-                        result.append(buffer);
-                        // XXX: sanitize string for ':' '='
-                        result.append(prop->u.CStringValue);
-                        buffer[0] = ':';
-                        buffer[1] = '\0';
-                        break;
-                default:
-                        ALOGE("to_String bad item type: %d for %s",
-                              prop->mType, prop->mName);
-                        break;
-            }
-            result.append(buffer);
+        mProps[i].toString(buffer, sizeof(buffer));
+        result.append(buffer);
     }
 
     if (version == PROTO_V0) {
@@ -984,12 +621,12 @@
             // no oprop, so we insert the new one
             oprop = allocateProp(p);
             if (oprop != NULL) {
-                copyProp(oprop, iprop);
+                *oprop = *iprop;
             } else {
                 ALOGW("dropped property '%s'", iprop->mName);
             }
         } else {
-            copyProp(oprop, iprop);
+            *oprop = *iprop;
         }
     }
 
@@ -1162,5 +799,58 @@
     return false;
 }
 
+void MediaAnalyticsItem::Prop::writeToParcel(Parcel *data) const
+{
+   data->writeCString(mName);
+   data->writeInt32(mType);
+   switch (mType) {
+   case kTypeInt32:
+       data->writeInt32(u.int32Value);
+       break;
+   case kTypeInt64:
+       data->writeInt64(u.int64Value);
+       break;
+   case kTypeDouble:
+       data->writeDouble(u.doubleValue);
+       break;
+   case kTypeRate:
+       data->writeInt64(u.rate.count);
+       data->writeInt64(u.rate.duration);
+       break;
+   case kTypeCString:
+       data->writeCString(u.CStringValue);
+       break;
+   default:
+       ALOGE("%s: found bad type: %d, name %s", __func__, mType, mName);
+       break;
+   }
+}
+
+void MediaAnalyticsItem::Prop::toString(char *buffer, size_t length) const {
+    switch (mType) {
+    case kTypeInt32:
+        snprintf(buffer, length, "%s=%d:", mName, u.int32Value);
+        break;
+    case MediaAnalyticsItem::kTypeInt64:
+        snprintf(buffer, length, "%s=%lld:", mName, (long long)u.int64Value);
+        break;
+    case MediaAnalyticsItem::kTypeDouble:
+        snprintf(buffer, length, "%s=%e:", mName, u.doubleValue);
+        break;
+    case MediaAnalyticsItem::kTypeRate:
+        snprintf(buffer, length, "%s=%lld/%lld:",
+                mName, (long long)u.rate.count, (long long)u.rate.duration);
+        break;
+    case MediaAnalyticsItem::kTypeCString:
+        // TODO sanitize string for ':' '='
+        snprintf(buffer, length, "%s=%s:", mName, u.CStringValue);
+        break;
+    default:
+        ALOGE("%s: bad item type: %d for %s", __func__, mType, mName);
+        if (length > 0) buffer[0] = 0;
+        break;
+    }
+}
+
 } // namespace android
 
diff --git a/media/libmediametrics/include/MediaAnalyticsItem.h b/media/libmediametrics/include/MediaAnalyticsItem.h
index b37eff43..f0deaaf 100644
--- a/media/libmediametrics/include/MediaAnalyticsItem.h
+++ b/media/libmediametrics/include/MediaAnalyticsItem.h
@@ -38,14 +38,10 @@
 //
 
 class MediaAnalyticsItem {
+    friend class MediaMetricsJNI;           // TODO: remove this access
+    friend class MediaMetricsDeathNotifier; // for dropInstance
 
-    friend class MediaAnalyticsService;
-    friend class IMediaAnalyticsService;
-    friend class MediaMetricsJNI;
-    friend class MetricsSummarizer;
-    friend class MediaMetricsDeathNotifier;
-
-    public:
+public:
 
             enum Type {
                 kTypeNone = 0,
@@ -56,14 +52,14 @@
                 kTypeRate = 5,
             };
 
-        // Key: the record descriminator
-        // values for the record discriminator
-        // values can be "component/component"
-        // basic values: "video", "audio", "drm"
-        // XXX: need to better define the format
-        typedef std::string Key;
-        static const Key kKeyNone;              // ""
-        static const Key kKeyAny;               // "*"
+    // Key: the record descriminator
+    // values for the record discriminator
+    // values can be "component/component"
+    // basic values: "video", "audio", "drm"
+    // XXX: need to better define the format
+    using Key = std::string;
+    static constexpr const char * const kKeyNone = "none";
+    static constexpr const char * const kKeyAny = "any";
 
         // Attr: names for attributes within a record
         // format "prop1" or "prop/subprop"
@@ -78,14 +74,12 @@
             PROTO_LAST = PROTO_V1,
         };
 
-    private:
-        // use the ::create() method instead
-        MediaAnalyticsItem();
-        MediaAnalyticsItem(Key);
-        MediaAnalyticsItem(const MediaAnalyticsItem&);
-        MediaAnalyticsItem &operator=(const MediaAnalyticsItem&);
-
-    public:
+    // T must be convertible to mKey
+    template <typename T>
+    explicit MediaAnalyticsItem(T key)
+        : mKey(key) { }
+    MediaAnalyticsItem(const MediaAnalyticsItem&) = delete;
+    MediaAnalyticsItem &operator=(const MediaAnalyticsItem&) = delete;
 
         static MediaAnalyticsItem* create(Key key);
         static MediaAnalyticsItem* create();
@@ -108,30 +102,89 @@
         // # of attributes in the record
         int32_t count() const;
 
-        // set values appropriately
-        void setInt32(Attr, int32_t value);
-        void setInt64(Attr, int64_t value);
-        void setDouble(Attr, double value);
-        void setRate(Attr, int64_t count, int64_t duration);
-        void setCString(Attr, const char *value);
+    template<typename S, typename T>
+    MediaAnalyticsItem &set(S key, T value) {
+        allocateProp(key)->set(value);
+        return *this;
+    }
 
-        // fused get/add/set; if attr wasn't there, it's a simple set.
-        // type-mismatch counts as "wasn't there".
-        void addInt32(Attr, int32_t value);
-        void addInt64(Attr, int64_t value);
-        void addDouble(Attr, double value);
-        void addRate(Attr, int64_t count, int64_t duration);
+    // set values appropriately
+    MediaAnalyticsItem &setInt32(Attr key, int32_t value) {
+        return set(key, value);
+    }
+    MediaAnalyticsItem &setInt64(Attr key, int64_t value) {
+        return set(key, value);
+    }
+    MediaAnalyticsItem &setDouble(Attr key, double value) {
+        return set(key, value);
+    }
+    MediaAnalyticsItem &setRate(Attr key, int64_t count, int64_t duration) {
+        return set(key, std::make_pair(count, duration));
+    }
+    MediaAnalyticsItem &setCString(Attr key, const char *value) {
+        return set(key, value);
+    }
 
-        // find & extract values
-        // return indicates whether attr exists (and thus value filled in)
-        // NULL parameter value suppresses storage of value.
-        bool getInt32(Attr, int32_t *value);
-        bool getInt64(Attr, int64_t *value);
-        bool getDouble(Attr, double *value);
-        bool getRate(Attr, int64_t *count, int64_t *duration, double *rate);
-        // Caller owns the returned string
-        bool getCString(Attr, char **value);
-        bool getString(Attr, std::string *value);
+    // fused get/add/set; if attr wasn't there, it's a simple set.
+    // type-mismatch counts as "wasn't there".
+    template<typename S, typename T>
+    MediaAnalyticsItem &add(S key, T value) {
+        allocateProp(key)->add(value);
+        return *this;
+    }
+
+    MediaAnalyticsItem &addInt32(Attr key, int32_t value) {
+        return add(key, value);
+    }
+    MediaAnalyticsItem &addInt64(Attr key, int64_t value) {
+        return add(key, value);
+    }
+    MediaAnalyticsItem &addDouble(Attr key, double value) {
+        return add(key, value);
+    }
+    MediaAnalyticsItem &addRate(Attr key, int64_t count, int64_t duration) {
+        return add(key, std::make_pair(count, duration));
+    }
+
+    // find & extract values
+    // return indicates whether attr exists (and thus value filled in)
+    // NULL parameter value suppresses storage of value.
+    template<typename S, typename T>
+    bool get(S key, T *value) const {
+        Prop *prop = findProp(key);
+        return prop != nullptr && prop->get(value);
+    }
+
+    bool getInt32(Attr key, int32_t *value) const {
+        return get(key, value);
+    }
+    bool getInt64(Attr key, int64_t *value) const {
+        return get(key, value);
+    }
+    bool getDouble(Attr key, double *value) const {
+        return get(key, value);
+    }
+    bool getRate(Attr key, int64_t *count, int64_t *duration, double *rate) const {
+        std::pair<int64_t, int64_t> value;
+        if (!get(key, &value)) return false;
+        if (count != nullptr) *count = value.first;
+        if (duration != nullptr) *duration = value.second;
+        if (rate != nullptr) {
+            if (value.second != 0) {
+                *rate = (double)value.first / value.second;  // TODO: isn't INF OK?
+            } else {
+                *rate = 0.;
+            }
+        }
+        return true;
+    }
+    // Caller owns the returned string
+    bool getCString(Attr key, char **value) const {
+        return get(key, value);
+    }
+    bool getString(Attr key, std::string *value) const {
+        return get(key, value);
+    }
 
         // Deliver the item to MediaMetrics
         bool selfrecord();
@@ -193,60 +246,261 @@
         // caller continues to own 'incoming'
         bool merge(MediaAnalyticsItem *incoming);
 
-        // enabled 1, disabled 0
-        static const char * const EnabledProperty;
-        static const char * const EnabledPropertyPersist;
-        static const int   EnabledProperty_default;
+    // enabled 1, disabled 0
+    static constexpr const char * const EnabledProperty = "media.metrics.enabled";
+    static constexpr const char * const EnabledPropertyPersist = "persist.media.metrics.enabled";
+    static const int EnabledProperty_default = 1;
 
     private:
 
-        // to help validate that A doesn't mess with B's records
-        pid_t     mPid;
-        uid_t     mUid;
-        std::string   mPkgName;
-        int64_t   mPkgVersionCode;
+    // let's reuse a binder connection
+    static sp<IMediaAnalyticsService> sAnalyticsService;
+    static sp<IMediaAnalyticsService> getInstance();
+    static void dropInstance();
 
-        // let's reuse a binder connection
-        static sp<IMediaAnalyticsService> sAnalyticsService;
-        static sp<IMediaAnalyticsService> getInstance();
-        static void dropInstance();
+    class Prop {
+    friend class MediaMetricsJNI;           // TODO: remove this access
+    public:
+        Prop() = default;
+        Prop(const Prop& other) {
+           *this = other;
+        }
+        Prop& operator=(const Prop& other) {
+            if (other.mName != nullptr) {
+                mName = strdup(other.mName);
+            } else {
+                mName = nullptr;
+            }
+            mNameLen = other.mNameLen;
+            mType = other.mType;
+            switch (mType) {
+            case kTypeInt32:
+                u.int32Value = other.u.int32Value;
+                break;
+            case kTypeInt64:
+                u.int64Value = other.u.int64Value;
+                break;
+            case kTypeDouble:
+                u.doubleValue = other.u.doubleValue;
+                break;
+            case kTypeCString:
+                u.CStringValue = strdup(other.u.CStringValue);
+                break;
+            case kTypeRate:
+                u.rate = {other.u.rate.count, other.u.rate.duration};
+                break;
+            case kTypeNone:
+                break;
+            default:
+                // abort?
+                break;
+            }
+            return *this;
+        }
 
-        // tracking information
-        nsecs_t mTimestamp;             // ns, system_time_monotonic
+        void clear() {
+            free(mName);
+            mName = nullptr;
+            mNameLen = 0;
+            clearValue();
+        }
+        void clearValue() {
+            if (mType == kTypeCString) {
+                free(u.CStringValue);
+                u.CStringValue = nullptr;
+            }
+            mType = kTypeNone;
+        }
 
-        Key mKey;
+        Type getType() const {
+            return mType;
+        }
 
-        struct Prop {
+        const char *getName() const {
+            return mName;
+        }
 
-            Type mType;
-            const char *mName;
-            size_t mNameLen;    // the strlen(), doesn't include the null
-            union {
-                    int32_t int32Value;
-                    int64_t int64Value;
-                    double doubleValue;
-                    char *CStringValue;
-                    struct { int64_t count, duration; } rate;
-            } u;
-            void setName(const char *name, size_t len);
-        };
+        void swap(Prop& other) {
+            std::swap(mName, other.mName);
+            std::swap(mNameLen, other.mNameLen);
+            std::swap(mType, other.mType);
+            std::swap(u, other.u);
+        }
 
-        void initProp(Prop *item);
-        void clearProp(Prop *item);
-        void clearPropValue(Prop *item);
-        void copyProp(Prop *dst, const Prop *src);
+        void setName(const char *name, size_t len) {
+            free(mName);
+            if (name != nullptr) {
+                mName = (char *)malloc(len + 1);
+                mNameLen = len;
+                strncpy(mName, name, len);
+                mName[len] = 0;
+            } else {
+                mName = nullptr;
+                mNameLen = 0;
+            }
+        }
+
+        bool isNamed(const char *name, size_t len) const {
+            return len == mNameLen && memcmp(name, mName, len) == 0;
+        }
+
+        // TODO: remove duplicate but different definition
+        bool isNamed(const char *name) const {
+            return strcmp(name, mName) == 0;
+        }
+
+        template <typename T> bool get(T *value) const = delete;
+        template <>
+        bool get(int32_t *value) const {
+           if (mType != kTypeInt32) return false;
+           if (value != nullptr) *value = u.int32Value;
+           return true;
+        }
+        template <>
+        bool get(int64_t *value) const {
+           if (mType != kTypeInt64) return false;
+           if (value != nullptr) *value = u.int64Value;
+           return true;
+        }
+        template <>
+        bool get(double *value) const {
+           if (mType != kTypeDouble) return false;
+           if (value != nullptr) *value = u.doubleValue;
+           return true;
+        }
+        template <>
+        bool get(char** value) const {
+            if (mType != kTypeCString) return false;
+            if (value != nullptr) *value = strdup(u.CStringValue);
+            return true;
+        }
+        template <>
+        bool get(std::string* value) const {
+            if (mType != kTypeCString) return false;
+            if (value != nullptr) *value = u.CStringValue;
+            return true;
+        }
+        template <>
+        bool get(std::pair<int64_t, int64_t> *value) const {
+           if (mType != kTypeRate) return false;
+           if (value != nullptr) {
+               value->first = u.rate.count;
+               value->second = u.rate.duration;
+           }
+           return true;
+        }
+
+        template <typename T> void set(const T& value) = delete;
+        template <>
+        void set(const int32_t& value) {
+            mType = kTypeInt32;
+            u.int32Value = value;
+        }
+        template <>
+        void set(const int64_t& value) {
+            mType = kTypeInt64;
+            u.int64Value = value;
+        }
+        template <>
+        void set(const double& value) {
+            mType = kTypeDouble;
+            u.doubleValue = value;
+        }
+        template <>
+        void set(const char* const& value) {
+            if (mType == kTypeCString) {
+                free(u.CStringValue);
+            } else {
+                mType = kTypeCString;
+            }
+            if (value == nullptr) {
+                u.CStringValue = nullptr;
+            } else {
+                u.CStringValue = strdup(value);
+            }
+        }
+        template <>
+        void set(const std::pair<int64_t, int64_t> &value) {
+            mType = kTypeRate;
+            u.rate = {value.first, value.second};
+        }
+
+        template <typename T> void add(const T& value) = delete;
+        template <>
+        void add(const int32_t& value) {
+            if (mType == kTypeInt32) {
+                u.int32Value += value;
+            } else {
+                mType = kTypeInt32;
+                u.int32Value = value;
+            }
+        }
+        template <>
+        void add(const int64_t& value) {
+            if (mType == kTypeInt64) {
+                u.int64Value += value;
+            } else {
+                mType = kTypeInt64;
+                u.int64Value = value;
+            }
+        }
+        template <>
+        void add(const double& value) {
+            if (mType == kTypeDouble) {
+                u.doubleValue += value;
+            } else {
+                mType = kTypeDouble;
+                u.doubleValue = value;
+            }
+        }
+        template <>
+        void add(const std::pair<int64_t, int64_t>& value) {
+            if (mType == kTypeRate) {
+                u.rate.count += value.first;
+                u.rate.duration += value.second;
+            } else {
+                mType = kTypeRate;
+                u.rate = {value.first, value.second};
+            }
+        }
+
+        void writeToParcel(Parcel *data) const;
+        void toString(char *buffer, size_t length) const;
+
+    // TODO: make private
+    // private:
+        char *mName = nullptr;
+        size_t mNameLen = 0;    // the strlen(), doesn't include the null
+        Type mType = kTypeNone;
+        union {
+            int32_t int32Value;
+            int64_t int64Value;
+            double doubleValue;
+            char *CStringValue;
+            struct { int64_t count, duration; } rate;
+        } u;
+    };
+
+    size_t findPropIndex(const char *name, size_t len) const;
+    Prop *findProp(const char *name) const;
+
         enum {
             kGrowProps = 10
         };
         bool growProps(int increment = kGrowProps);
-        size_t findPropIndex(const char *name, size_t len);
-        Prop *findProp(const char *name);
         Prop *allocateProp(const char *name);
         bool removeProp(const char *name);
 
-        size_t mPropCount;
-        size_t mPropSize;
-        Prop *mProps;
+        size_t mPropCount = 0;
+        size_t mPropSize = 0;
+        Prop *mProps = nullptr;
+
+    pid_t         mPid = -1;
+    uid_t         mUid = -1;
+    std::string   mPkgName;
+    int64_t       mPkgVersionCode = 0;
+    Key           mKey{kKeyNone};
+    nsecs_t       mTimestamp = 0;
 };
 
 } // namespace android
diff --git a/services/mediaanalytics/tests/mediametrics_tests.cpp b/services/mediaanalytics/tests/mediametrics_tests.cpp
index ea7739b..7a6f5a4 100644
--- a/services/mediaanalytics/tests/mediametrics_tests.cpp
+++ b/services/mediaanalytics/tests/mediametrics_tests.cpp
@@ -50,5 +50,128 @@
   status = mediaMetrics->submit(audiotrack_key.get());
   ASSERT_EQ(NO_ERROR, status);
 
+
+  /*
+  // fluent style that goes directly to mediametrics
+  ASSERT_EQ(true, MediaAnalyticsItem("audiorecord")
+                     .setInt32("value", 2)
+                     .addInt32("bar", 1)
+                     .addInt32("value", 3)
+                     .selfrecord());
+  */
+
   mediaMetrics->dump(fileno(stdout), {} /* args */);
 }
+
+TEST(mediametrics_tests, item_manipulation) {
+  MediaAnalyticsItem item("audiorecord");
+
+  item.setInt32("value", 2).addInt32("bar", 3).addInt32("value", 4);
+
+  int32_t i32;
+  ASSERT_TRUE(item.getInt32("value", &i32));
+  ASSERT_EQ(6, i32);
+
+  ASSERT_TRUE(item.getInt32("bar", &i32));
+  ASSERT_EQ(3, i32);
+
+  item.setInt64("big", INT64_MAX).setInt64("smaller", INT64_MAX - 1).addInt64("smaller", -2);
+
+  int64_t i64;
+  ASSERT_TRUE(item.getInt64("big", &i64));
+  ASSERT_EQ(INT64_MAX, i64);
+
+  ASSERT_TRUE(item.getInt64("smaller", &i64));
+  ASSERT_EQ(INT64_MAX - 3, i64);
+
+  item.setDouble("precise", 10.5).setDouble("small", 0.125).addDouble("precise", 0.25);
+
+  double d;
+  ASSERT_TRUE(item.getDouble("precise", &d));
+  ASSERT_EQ(10.75, d);
+
+  ASSERT_TRUE(item.getDouble("small", &d));
+  ASSERT_EQ(0.125, d);
+
+  char *s;
+  item.setCString("name", "Frank").setCString("mother", "June").setCString("mother", "July");
+  ASSERT_TRUE(item.getCString("name", &s));
+  ASSERT_EQ(0, strcmp(s, "Frank"));
+  free(s);
+
+  ASSERT_TRUE(item.getCString("mother", &s));
+  ASSERT_EQ(0, strcmp(s, "July"));  // "July" overwrites "June"
+  free(s);
+
+  item.setRate("burgersPerHour", 5, 2);
+  int64_t b, h;
+  ASSERT_TRUE(item.getRate("burgersPerHour", &b, &h, &d));
+  ASSERT_EQ(5, b);
+  ASSERT_EQ(2, h);
+  ASSERT_EQ(2.5, d);
+
+  item.addRate("burgersPerHour", 4, 2);
+  ASSERT_TRUE(item.getRate("burgersPerHour", &b, &h, &d));
+  ASSERT_EQ(9, b);
+  ASSERT_EQ(4, h);
+  ASSERT_EQ(2.25, d);
+
+  printf("item: %s\n", item.toString().c_str());
+  fflush(stdout);
+
+  sp mediaMetrics = new MediaAnalyticsService();
+  status_t status = mediaMetrics->submit(&item);
+  ASSERT_EQ(NO_ERROR, status);
+  mediaMetrics->dump(fileno(stdout), {} /* args */);
+}
+
+TEST(mediametrics_tests, superbig_item) {
+  MediaAnalyticsItem item("TheBigOne");
+  constexpr size_t count = 10000;
+
+  for (size_t i = 0; i < count; ++i) {
+    item.setInt32(std::to_string(i).c_str(), i);
+  }
+  for (size_t i = 0; i < count; ++i) {
+    int32_t i32;
+    ASSERT_TRUE(item.getInt32(std::to_string(i).c_str(), &i32));
+    ASSERT_EQ((int32_t)i, i32);
+  }
+}
+
+TEST(mediametrics_tests, superbig_item_removal) {
+  MediaAnalyticsItem item("TheOddBigOne");
+  constexpr size_t count = 10000;
+
+  for (size_t i = 0; i < count; ++i) {
+    item.setInt32(std::to_string(i).c_str(), i);
+  }
+  for (size_t i = 0; i < count; i += 2) {
+    item.filter(std::to_string(i).c_str()); // filter out all the evens.
+  }
+  for (size_t i = 0; i < count; ++i) {
+    int32_t i32;
+    if (i & 1) { // check to see that only the odds are left.
+        ASSERT_TRUE(item.getInt32(std::to_string(i).c_str(), &i32));
+        ASSERT_EQ((int32_t)i, i32);
+    } else {
+        ASSERT_FALSE(item.getInt32(std::to_string(i).c_str(), &i32));
+    }
+  }
+}
+
+TEST(mediametrics_tests, item_transmutation) {
+  MediaAnalyticsItem item("Alchemist's Stone");
+
+  item.setInt64("convert", 123);
+  int64_t i64;
+  ASSERT_TRUE(item.getInt64("convert", &i64));
+  ASSERT_EQ(123, i64);
+
+  item.addInt32("convert", 2);     // changes type of 'convert' from i64 to i32 (and re-init).
+  ASSERT_FALSE(item.getInt64("convert", &i64));  // should be false, no value in i64.
+
+  int32_t i32;
+  ASSERT_TRUE(item.getInt32("convert", &i32));   // check it is i32 and 2 (123 is discarded).
+  ASSERT_EQ(2, i32);
+}