Merge "Wrap StatsDimensionsValue over structured parcel" into rvc-dev am: 875a90282a am: bf8cb7b572

Change-Id: I665f351f8c5e84b7a9de88d13cba476d89601ef9

apex/statsd/aidl/android/os/StatsDimensionsValueParcel.aidl

@@ -4,12 +4,12 @@ package android.os;
  * @hide
  */
 parcelable StatsDimensionsValueParcel {
-    /**
+    // Field equals atomTag for top level StatsDimensionsValueParcels or
-     * Field equals:
+    // positions in depth (1-indexed) for lower level parcels.
-     *      - atomTag for top level StatsDimensionsValueParcel
-     *      - position in dimension for all other levels
-     */
     int field;
+
+    // Indicator for which type of value is stored. Should be set to one
+    // of the constants in StatsDimensionsValue.java.
     int valueType;
 
     String stringValue;

apex/statsd/framework/java/android/os/StatsDimensionsValue.java

@@ -58,7 +58,7 @@ public final class StatsDimensionsValue implements Parcelable {
     private static final String TAG = "StatsDimensionsValue";
 
     // Values of the value type correspond to stats_log.proto's DimensionValue fields.
-    // Keep constants in sync with services/include/android/os/StatsDimensionsValue.h.
+    // Keep constants in sync with frameworks/base/cmds/statsd/src/HashableDimensionKey.cpp.
     /** Indicates that this holds a String. */
     public static final int STRING_VALUE_TYPE = 2;
     /** Indicates that this holds an int. */
@@ -72,17 +72,7 @@ public final class StatsDimensionsValue implements Parcelable {
     /** Indicates that this holds a List of StatsDimensionsValues. */
     public static final int TUPLE_VALUE_TYPE = 7;
 
-    /** Value of a stats_log.proto DimensionsValue.field. */
+    private final StatsDimensionsValueParcel mInner;
-    private final int mField;
-
-    /** Type of stats_log.proto DimensionsValue.value, according to the VALUE_TYPEs above. */
-    private final int mValueType;
-
-    /**
-     * Value of a stats_log.proto DimensionsValue.value.
-     * String, Integer, Long, Boolean, Float, or StatsDimensionsValue[].
-     */
-    private final Object mValue; // immutable or array of immutables
 
     /**
      * Creates a {@code StatsDimensionValue} from a parcel.
@@ -90,59 +80,25 @@ public final class StatsDimensionsValue implements Parcelable {
      * @hide
      */
     public StatsDimensionsValue(Parcel in) {
-        mField = in.readInt();
+        mInner = StatsDimensionsValueParcel.CREATOR.createFromParcel(in);
-        mValueType = in.readInt();
-        mValue = readValueFromParcel(mValueType, in);
     }
 
     /**
      * Creates a {@code StatsDimensionsValue} from a StatsDimensionsValueParcel
-     * TODO(b/149103391): Make StatsDimensionsValue a wrapper on top of
-     * StatsDimensionsValueParcel.
      *
      * @hide
      */
     public StatsDimensionsValue(StatsDimensionsValueParcel parcel) {
-        mField = parcel.field;
+        mInner = parcel;
-        mValueType = parcel.valueType;
-        switch (mValueType) {
-            case STRING_VALUE_TYPE:
-                mValue = parcel.stringValue;
-                break;
-            case INT_VALUE_TYPE:
-                mValue = parcel.intValue;
-                break;
-            case LONG_VALUE_TYPE:
-                mValue = parcel.longValue;
-                break;
-            case BOOLEAN_VALUE_TYPE:
-                mValue = parcel.boolValue;
-                break;
-            case FLOAT_VALUE_TYPE:
-                mValue = parcel.floatValue;
-                break;
-            case TUPLE_VALUE_TYPE:
-                StatsDimensionsValue[] values = new StatsDimensionsValue[parcel.tupleValue.length];
-                for (int i = 0; i < parcel.tupleValue.length; i++) {
-                    values[i] = new StatsDimensionsValue(parcel.tupleValue[i]);
-                }
-                mValue = values;
-                break;
-            default:
-                Log.w(TAG, "StatsDimensionsValueParcel contains bad valueType: " + mValueType);
-                mValue = null;
-                break;
-        }
     }
 
-
     /**
      * Return the field, i.e. the tag of a statsd atom.
      *
      * @return the field
      */
     public int getField() {
-        return mField;
+        return mInner.field;
     }
 
     /**
@@ -152,12 +108,12 @@ public final class StatsDimensionsValue implements Parcelable {
      *         null otherwise
      */
     public String getStringValue() {
-        try {
+        if (mInner.valueType == STRING_VALUE_TYPE) {
-            if (mValueType == STRING_VALUE_TYPE) return (String) mValue;
+            return mInner.stringValue;
-        } catch (ClassCastException e) {
+        } else {
-            Log.w(TAG, "Failed to successfully get value", e);
+            Log.w(TAG, "Value type is " + getValueTypeAsString() + ", not string.");
+            return null;
         }
-        return null;
     }
 
     /**
@@ -166,12 +122,12 @@ public final class StatsDimensionsValue implements Parcelable {
      * @return the int held if {@link #getValueType()} == {@link #INT_VALUE_TYPE}, 0 otherwise
      */
     public int getIntValue() {
-        try {
+        if (mInner.valueType == INT_VALUE_TYPE) {
-            if (mValueType == INT_VALUE_TYPE) return (Integer) mValue;
+            return mInner.intValue;
-        } catch (ClassCastException e) {
+        } else {
-            Log.w(TAG, "Failed to successfully get value", e);
+            Log.w(TAG, "Value type is " + getValueTypeAsString() + ", not int.");
+            return 0;
         }
-        return 0;
     }
 
     /**
@@ -180,12 +136,12 @@ public final class StatsDimensionsValue implements Parcelable {
      * @return the long held if {@link #getValueType()} == {@link #LONG_VALUE_TYPE}, 0 otherwise
      */
     public long getLongValue() {
-        try {
+        if (mInner.valueType == LONG_VALUE_TYPE) {
-            if (mValueType == LONG_VALUE_TYPE) return (Long) mValue;
+            return mInner.longValue;
-        } catch (ClassCastException e) {
+        } else {
-            Log.w(TAG, "Failed to successfully get value", e);
+            Log.w(TAG, "Value type is " + getValueTypeAsString() + ", not long.");
+            return 0;
         }
-        return 0;
     }
 
     /**
@@ -195,12 +151,12 @@ public final class StatsDimensionsValue implements Parcelable {
      *         false otherwise
      */
     public boolean getBooleanValue() {
-        try {
+        if (mInner.valueType == BOOLEAN_VALUE_TYPE) {
-            if (mValueType == BOOLEAN_VALUE_TYPE) return (Boolean) mValue;
+            return mInner.boolValue;
-        } catch (ClassCastException e) {
+        } else {
-            Log.w(TAG, "Failed to successfully get value", e);
+            Log.w(TAG, "Value type is " + getValueTypeAsString() + ", not boolean.");
+            return false;
         }
-        return false;
     }
 
     /**
@@ -209,12 +165,12 @@ public final class StatsDimensionsValue implements Parcelable {
      * @return the float held if {@link #getValueType()} == {@link #FLOAT_VALUE_TYPE}, 0 otherwise
      */
     public float getFloatValue() {
-        try {
+        if (mInner.valueType == FLOAT_VALUE_TYPE) {
-            if (mValueType == FLOAT_VALUE_TYPE) return (Float) mValue;
+            return mInner.floatValue;
-        } catch (ClassCastException e) {
+        } else {
-            Log.w(TAG, "Failed to successfully get value", e);
+            Log.w(TAG, "Value type is " + getValueTypeAsString() + ", not float.");
+            return 0;
         }
-        return 0;
     }
 
     /**
@@ -226,19 +182,15 @@ public final class StatsDimensionsValue implements Parcelable {
      *         null otherwise
      */
     public List<StatsDimensionsValue> getTupleValueList() {
-        if (mValueType != TUPLE_VALUE_TYPE) {
+        if (mInner.valueType == TUPLE_VALUE_TYPE) {
-            return null;
+            int length = (mInner.tupleValue == null) ? 0 : mInner.tupleValue.length;
-        }
+            List<StatsDimensionsValue> tupleValues = new ArrayList<>(length);
-        try {
+            for (int i = 0; i < length; i++) {
-            StatsDimensionsValue[] orig = (StatsDimensionsValue[]) mValue;
+                tupleValues.add(new StatsDimensionsValue(mInner.tupleValue[i]));
-            List<StatsDimensionsValue> copy = new ArrayList<>(orig.length);
-            // Shallow copy since StatsDimensionsValue is immutable anyway
-            for (int i = 0; i < orig.length; i++) {
-                copy.add(orig[i]);
             }
-            return copy;
+            return tupleValues;
-        } catch (ClassCastException e) {
+        } else {
-            Log.w(TAG, "Failed to successfully get value", e);
+            Log.w(TAG, "Value type is " + getValueTypeAsString() + ", not tuple.");
             return null;
         }
     }
@@ -257,7 +209,7 @@ public final class StatsDimensionsValue implements Parcelable {
      * @return the constant representing the type of value stored
      */
     public int getValueType() {
-        return mValueType;
+        return mInner.valueType;
     }
 
     /**
@@ -267,7 +219,7 @@ public final class StatsDimensionsValue implements Parcelable {
      * @return true if {@link #getValueType()} is equal to {@code valueType}.
      */
     public boolean isValueType(int valueType) {
-        return mValueType == valueType;
+        return mInner.valueType == valueType;
     }
 
     /**
@@ -280,26 +232,40 @@ public final class StatsDimensionsValue implements Parcelable {
      */
     // Follows the format of statsd's dimension.h toString.
     public String toString() {
-        try {
+        StringBuilder sb = new StringBuilder();
-            StringBuilder sb = new StringBuilder();
+        sb.append(mInner.field);
-            sb.append(mField);
+        sb.append(":");
-            sb.append(":");
+        switch (mInner.valueType) {
-            if (mValueType == TUPLE_VALUE_TYPE) {
+            case STRING_VALUE_TYPE:
+                sb.append(mInner.stringValue);
+                break;
+            case INT_VALUE_TYPE:
+                sb.append(String.valueOf(mInner.intValue));
+                break;
+            case LONG_VALUE_TYPE:
+                sb.append(String.valueOf(mInner.longValue));
+                break;
+            case BOOLEAN_VALUE_TYPE:
+                sb.append(String.valueOf(mInner.boolValue));
+                break;
+            case FLOAT_VALUE_TYPE:
+                sb.append(String.valueOf(mInner.floatValue));
+                break;
+            case TUPLE_VALUE_TYPE:
                 sb.append("{");
-                StatsDimensionsValue[] sbvs = (StatsDimensionsValue[]) mValue;
+                int length = (mInner.tupleValue == null) ? 0 : mInner.tupleValue.length;
-                for (int i = 0; i < sbvs.length; i++) {
+                for (int i = 0; i < length; i++) {
-                    sb.append(sbvs[i].toString());
+                    StatsDimensionsValue child = new StatsDimensionsValue(mInner.tupleValue[i]);
+                    sb.append(child.toString());
                     sb.append("|");
                 }
                 sb.append("}");
-            } else {
+                break;
-                sb.append(mValue.toString());
+            default:
-            }
+                Log.w(TAG, "Incorrect value type");
-            return sb.toString();
+                break;
-        } catch (ClassCastException e) {
-            Log.w(TAG, "Failed to successfully get value", e);
         }
-        return "";
+        return sb.toString();
     }
 
     /**
@@ -324,72 +290,28 @@ public final class StatsDimensionsValue implements Parcelable {
 
     @Override
     public void writeToParcel(Parcel out, int flags) {
-        out.writeInt(mField);
+        mInner.writeToParcel(out, flags);
-        out.writeInt(mValueType);
-        writeValueToParcel(mValueType, mValue, out, flags);
     }
 
-    /** Writes mValue to a parcel. Returns true if succeeds. */
+    /**
-    private static boolean writeValueToParcel(int valueType, Object value, Parcel out, int flags) {
+     * Returns a string representation of the type of value stored.
-        try {
+     */
-            switch (valueType) {
+    private String getValueTypeAsString() {
-                case STRING_VALUE_TYPE:
+        switch (mInner.valueType) {
-                    out.writeString((String) value);
-                    return true;
-                case INT_VALUE_TYPE:
-                    out.writeInt((Integer) value);
-                    return true;
-                case LONG_VALUE_TYPE:
-                    out.writeLong((Long) value);
-                    return true;
-                case BOOLEAN_VALUE_TYPE:
-                    out.writeBoolean((Boolean) value);
-                    return true;
-                case FLOAT_VALUE_TYPE:
-                    out.writeFloat((Float) value);
-                    return true;
-                case TUPLE_VALUE_TYPE: {
-                    StatsDimensionsValue[] values = (StatsDimensionsValue[]) value;
-                    out.writeInt(values.length);
-                    for (int i = 0; i < values.length; i++) {
-                        values[i].writeToParcel(out, flags);
-                    }
-                    return true;
-                }
-                default:
-                    Log.w(TAG, "readValue of an impossible type " + valueType);
-                    return false;
-            }
-        } catch (ClassCastException e) {
-            Log.w(TAG, "writeValue cast failed", e);
-            return false;
-        }
-    }
-
-    /** Reads mValue from a parcel. */
-    private static Object readValueFromParcel(int valueType, Parcel parcel) {
-        switch (valueType) {
             case STRING_VALUE_TYPE:
-                return parcel.readString();
+                return "string";
             case INT_VALUE_TYPE:
-                return parcel.readInt();
+                return "int";
             case LONG_VALUE_TYPE:
-                return parcel.readLong();
+                return "long";
             case BOOLEAN_VALUE_TYPE:
-                return parcel.readBoolean();
+                return "boolean";
             case FLOAT_VALUE_TYPE:
-                return parcel.readFloat();
+                return "float";
-            case TUPLE_VALUE_TYPE: {
+            case TUPLE_VALUE_TYPE:
-                final int sz = parcel.readInt();
+                return "tuple";
-                StatsDimensionsValue[] values = new StatsDimensionsValue[sz];
-                for (int i = 0; i < sz; i++) {
-                    values[i] = new StatsDimensionsValue(parcel);
-                }
-                return values;
-            }
             default:
-                Log.w(TAG, "readValue of an impossible type " + valueType);
+                return "unknown";
-                return null;
         }
     }
 }

apex/statsd/framework/test/src/android/os/StatsDimensionsValueTest.java

@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2019 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.
+ */
+
+package android.os;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.util.List;
+
+@RunWith(JUnit4.class)
+public final class StatsDimensionsValueTest {
+
+    @Test
+    public void testConversionFromStructuredParcel() {
+        int tupleField = 100; // atom id
+        String stringValue = "Hello";
+        int intValue = 123;
+        long longValue = 123456789L;
+        float floatValue = 1.1f;
+        boolean boolValue = true;
+
+        // Construct structured parcel
+        StatsDimensionsValueParcel sdvp = new StatsDimensionsValueParcel();
+        sdvp.field = tupleField;
+        sdvp.valueType = StatsDimensionsValue.TUPLE_VALUE_TYPE;
+        sdvp.tupleValue = new StatsDimensionsValueParcel[5];
+
+        for (int i = 0; i < 5; i++) {
+            sdvp.tupleValue[i] = new StatsDimensionsValueParcel();
+            sdvp.tupleValue[i].field = i + 1;
+        }
+
+        sdvp.tupleValue[0].valueType = StatsDimensionsValue.STRING_VALUE_TYPE;
+        sdvp.tupleValue[1].valueType = StatsDimensionsValue.INT_VALUE_TYPE;
+        sdvp.tupleValue[2].valueType = StatsDimensionsValue.LONG_VALUE_TYPE;
+        sdvp.tupleValue[3].valueType = StatsDimensionsValue.FLOAT_VALUE_TYPE;
+        sdvp.tupleValue[4].valueType = StatsDimensionsValue.BOOLEAN_VALUE_TYPE;
+
+        sdvp.tupleValue[0].stringValue = stringValue;
+        sdvp.tupleValue[1].intValue = intValue;
+        sdvp.tupleValue[2].longValue = longValue;
+        sdvp.tupleValue[3].floatValue = floatValue;
+        sdvp.tupleValue[4].boolValue = boolValue;
+
+        // Convert to StatsDimensionsValue and check result
+        StatsDimensionsValue sdv = new StatsDimensionsValue(sdvp);
+
+        assertThat(sdv.getField()).isEqualTo(tupleField);
+        assertThat(sdv.getValueType()).isEqualTo(StatsDimensionsValue.TUPLE_VALUE_TYPE);
+        List<StatsDimensionsValue> sdvChildren = sdv.getTupleValueList();
+        assertThat(sdvChildren.size()).isEqualTo(5);
+
+        for (int i = 0; i < 5; i++) {
+            assertThat(sdvChildren.get(i).getField()).isEqualTo(i + 1);
+        }
+
+        assertThat(sdvChildren.get(0).getValueType())
+              .isEqualTo(StatsDimensionsValue.STRING_VALUE_TYPE);
+        assertThat(sdvChildren.get(1).getValueType())
+              .isEqualTo(StatsDimensionsValue.INT_VALUE_TYPE);
+        assertThat(sdvChildren.get(2).getValueType())
+              .isEqualTo(StatsDimensionsValue.LONG_VALUE_TYPE);
+        assertThat(sdvChildren.get(3).getValueType())
+              .isEqualTo(StatsDimensionsValue.FLOAT_VALUE_TYPE);
+        assertThat(sdvChildren.get(4).getValueType())
+              .isEqualTo(StatsDimensionsValue.BOOLEAN_VALUE_TYPE);
+
+        assertThat(sdvChildren.get(0).getStringValue()).isEqualTo(stringValue);
+        assertThat(sdvChildren.get(1).getIntValue()).isEqualTo(intValue);
+        assertThat(sdvChildren.get(2).getLongValue()).isEqualTo(longValue);
+        assertThat(sdvChildren.get(3).getFloatValue()).isEqualTo(floatValue);
+        assertThat(sdvChildren.get(4).getBooleanValue()).isEqualTo(boolValue);
+
+        // Ensure that StatsDimensionsValue and StatsDimensionsValueParcel are
+        // parceled equivalently
+        Parcel sdvpParcel = Parcel.obtain();
+        Parcel sdvParcel = Parcel.obtain();
+        sdvp.writeToParcel(sdvpParcel, 0);
+        sdv.writeToParcel(sdvParcel, 0);
+        assertThat(sdvpParcel.dataSize()).isEqualTo(sdvParcel.dataSize());
+    }
+
+    @Test
+    public void testNullTupleArray() {
+        int tupleField = 100; // atom id
+
+        StatsDimensionsValueParcel parcel = new StatsDimensionsValueParcel();
+        parcel.field = tupleField;
+        parcel.valueType = StatsDimensionsValue.TUPLE_VALUE_TYPE;
+        parcel.tupleValue = null;
+
+        StatsDimensionsValue sdv = new StatsDimensionsValue(parcel);
+        assertThat(sdv.getField()).isEqualTo(tupleField);
+        assertThat(sdv.getValueType()).isEqualTo(StatsDimensionsValue.TUPLE_VALUE_TYPE);
+        List<StatsDimensionsValue> sdvChildren = sdv.getTupleValueList();
+        assertThat(sdvChildren.size()).isEqualTo(0);
+    }
+}