Minor improvements on ApiCallStats and ApiCallStatsTest.
ApiCallStats:
- Make it final
- Hide constructor
- Encapsulated fields
- Removed redundant @NonNull annotations
- Throws (ISE instead of IAE) when both app and sdk name are set
ApiCallStatsTest:
- Extends AdServiceUnitTestCase
- Uses Truth instead of JUnit assertions
- Added tests for equals() / hashcode() / toString()
- Split testIncompleteInputBuilder_throwsIAE in 2 tests
- Uses non-zero result code
Test: atest AdServicesServiceCoreUnitTests:ApiCallStatsTest
Bug: 270974848
Change-Id: I6d623cd549ccde12ba06d9bbfdd7b7fe4caf6666
diff --git a/adservices/service-core/java/com/android/adservices/service/stats/ApiCallStats.java b/adservices/service-core/java/com/android/adservices/service/stats/ApiCallStats.java
index 018cf37..34a6598 100644
--- a/adservices/service-core/java/com/android/adservices/service/stats/ApiCallStats.java
+++ b/adservices/service-core/java/com/android/adservices/service/stats/ApiCallStats.java
@@ -16,21 +16,19 @@
package com.android.adservices.service.stats;
-import android.annotation.NonNull;
-
import java.util.Objects;
/** Class for Api Call Stats. */
-public class ApiCallStats {
+public final class ApiCallStats {
private int mCode;
private int mApiClass;
- int mApiName;
- String mAppPackageName;
- String mSdkPackageName;
- int mLatencyMillisecond;
- int mResultCode;
+ private int mApiName;
+ private String mAppPackageName;
+ private String mSdkPackageName;
+ private int mLatencyMillisecond;
+ private int mResultCode;
- public ApiCallStats() {}
+ private ApiCallStats() {}
@Override
public boolean equals(Object obj) {
@@ -59,37 +57,30 @@
mResultCode);
}
- @NonNull
public int getCode() {
return mCode;
}
- @NonNull
public int getApiClass() {
return mApiClass;
}
- @NonNull
public int getApiName() {
return mApiName;
}
- @NonNull
public String getAppPackageName() {
return mAppPackageName;
}
- @NonNull
public String getSdkPackageName() {
return mSdkPackageName;
}
- @NonNull
public int getLatencyMillisecond() {
return mLatencyMillisecond;
}
- @NonNull
public int getResultCode() {
return mResultCode;
}
@@ -118,60 +109,60 @@
/** Builder for {@link ApiCallStats}. */
public static final class Builder {
- private final ApiCallStats mBuilding;
+ private final ApiCallStats mBuilding = new ApiCallStats();
public Builder() {
- mBuilding = new ApiCallStats();
}
/** See {@link ApiCallStats#getCode()} . */
- public @NonNull ApiCallStats.Builder setCode(int code) {
+ public Builder setCode(int code) {
mBuilding.mCode = code;
return this;
}
/** See {@link ApiCallStats#getApiClass()} . */
- public @NonNull ApiCallStats.Builder setApiClass(int apiClass) {
+ public Builder setApiClass(int apiClass) {
mBuilding.mApiClass = apiClass;
return this;
}
/** See {@link ApiCallStats#getApiName()} . */
- public @NonNull ApiCallStats.Builder setApiName(int apiName) {
+ public Builder setApiName(int apiName) {
mBuilding.mApiName = apiName;
return this;
}
/** See {@link ApiCallStats#getAppPackageName()} . */
- public @NonNull ApiCallStats.Builder setAppPackageName(@NonNull String appPackageName) {
- Objects.requireNonNull(appPackageName);
- mBuilding.mAppPackageName = appPackageName;
+ public Builder setAppPackageName(String appPackageName) {
+ mBuilding.mAppPackageName = Objects.requireNonNull(appPackageName);
return this;
}
/** See {@link ApiCallStats#getSdkPackageName()}. */
- public @NonNull ApiCallStats.Builder setSdkPackageName(@NonNull String sdkPackageName) {
- Objects.requireNonNull(sdkPackageName);
- mBuilding.mSdkPackageName = sdkPackageName;
+ public Builder setSdkPackageName(String sdkPackageName) {
+ mBuilding.mSdkPackageName = Objects.requireNonNull(sdkPackageName);
return this;
}
/** See {@link ApiCallStats#getLatencyMillisecond()}. */
- public @NonNull ApiCallStats.Builder setLatencyMillisecond(int latencyMillisecond) {
+ public Builder setLatencyMillisecond(int latencyMillisecond) {
mBuilding.mLatencyMillisecond = latencyMillisecond;
return this;
}
/** See {@link ApiCallStats#getResultCode()}. */
- public @NonNull ApiCallStats.Builder setResultCode(int resultCode) {
+ public Builder setResultCode(int resultCode) {
mBuilding.mResultCode = resultCode;
return this;
}
/** Build the {@link ApiCallStats}. */
- public @NonNull ApiCallStats build() {
- if (mBuilding.mAppPackageName == null || mBuilding.mSdkPackageName == null) {
- throw new IllegalArgumentException("appPackageName or sdkPackageName is null");
+ public ApiCallStats build() {
+ if (mBuilding.mAppPackageName == null) {
+ throw new IllegalStateException("must call setAppPackageName()");
+ }
+ if (mBuilding.mSdkPackageName == null) {
+ throw new IllegalStateException("must call setSdkPackageName()");
}
return mBuilding;
}
diff --git a/adservices/tests/test-util/java/com/android/adservices/common/AdServicesTestCase.java b/adservices/tests/test-util/java/com/android/adservices/common/AdServicesTestCase.java
index 6ec9ab3..090fea8 100644
--- a/adservices/tests/test-util/java/com/android/adservices/common/AdServicesTestCase.java
+++ b/adservices/tests/test-util/java/com/android/adservices/common/AdServicesTestCase.java
@@ -20,6 +20,7 @@
import android.os.SystemProperties;
import android.util.Log;
+import androidx.annotation.Nullable;
import androidx.test.InstrumentationRegistry;
import com.google.common.truth.Expect;
@@ -30,6 +31,8 @@
import org.junit.Before;
import org.junit.Rule;
+import java.util.Objects;
+
// TODO(b/285014040): need to add unit tests for this class itself, as it's now providing logic.
// Superclass for all other "base classes"
@@ -139,6 +142,43 @@
.start();
}
+ // TODO(b/285014040): refactor to take Object... instead (once it's unit tested)
+ /**
+ * Helper method that uses {@code expect} to assert the class properly implement {@code
+ * equals()} and {@code hashCode()}.
+ *
+ * @param obj1 object that is equals to {@code obj2}
+ * @param obj2 object that is equals to {@code obj1}
+ */
+ protected final void expectObjectsAreEqual(Object obj1, Object obj2) {
+ Objects.requireNonNull(obj1, "1st arg cannot be null");
+ Objects.requireNonNull(obj2, "2nd arg cannot be null");
+
+ expect.withMessage("1st obj (%s)", obj1).that(obj1).isEqualTo(obj2);
+ expect.withMessage("2nd obj (%s)", obj2).that(obj2).isEqualTo(obj1);
+ expect.withMessage("hashCode of %s", obj1).that(obj1.hashCode()).isEqualTo(obj2.hashCode());
+ }
+
+ // TODO(b/285014040): refactor to take Object... instead (once it's unit tested)
+ /**
+ * Helper method that uses {@code expect} to assert the class properly implement {@code
+ * equals()} and {@code hashCode()}.
+ *
+ * @param obj1 object that is not equal to {@code obj2}
+ * @param obj2 object that is not equal to {@code obj1}
+ */
+ protected final void expectObjectsAreNotEqual(Object obj1, @Nullable Object obj2) {
+ Objects.requireNonNull(obj1, "1st arg cannot be null");
+
+ expect.withMessage("1st obj (%s)", obj1).that(obj1).isNotEqualTo(obj2);
+ expect.withMessage("2nd obj (%s)", obj2).that(obj2).isNotEqualTo(obj1);
+ if (obj2 != null) {
+ expect.withMessage("hashCode of %s", obj1)
+ .that(obj1.hashCode())
+ .isNotEqualTo(obj2.hashCode());
+ }
+ }
+
private void sleepAfterTest() {
int napTimeMs = SystemProperties.getInt(PROP_DELAY_AFTER_TEST, 0);
if (napTimeMs <= 0) {
diff --git a/adservices/tests/unittest/service-core/src/com/android/adservices/service/stats/ApiCallStatsTest.java b/adservices/tests/unittest/service-core/src/com/android/adservices/service/stats/ApiCallStatsTest.java
index 82cfc4d..e2f2a4a 100644
--- a/adservices/tests/unittest/service-core/src/com/android/adservices/service/stats/ApiCallStatsTest.java
+++ b/adservices/tests/unittest/service-core/src/com/android/adservices/service/stats/ApiCallStatsTest.java
@@ -16,74 +16,235 @@
package com.android.adservices.service.stats;
-import static com.android.adservices.ResultCode.RESULT_OK;
+import static android.adservices.common.AdServicesStatusUtils.STATUS_INVALID_ARGUMENT;
+
import static com.android.adservices.service.stats.AdServicesStatsLog.AD_SERVICES_API_CALLED;
import static com.android.adservices.service.stats.AdServicesStatsLog.AD_SERVICES_API_CALLED__API_CLASS__TARGETING;
import static com.android.adservices.service.stats.AdServicesStatsLog.AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS;
-import org.junit.Assert;
+import static org.junit.Assert.assertThrows;
+
+import com.android.adservices.common.AdServicesUnitTestCase;
+
import org.junit.Test;
-/**
- * Unit test for {@link ApiCallStats}.
- */
-public class ApiCallStatsTest {
+/** Unit test for {@link ApiCallStats}. */
+public final class ApiCallStatsTest extends AdServicesUnitTestCase {
- private final String mPackageName = "com.android.test";
- private final String mSdkName = "com.android.container";
- private static final int LATENCY = 100;
+ private static final int LATENCY_MS = 100;
+
+ private final String mAppPackageName = "com.android.test";
+ private final String mSdkPackageName = "com.android.container";
+
@Test
public void testBuilderCreateSuccess() {
- ApiCallStats stats = new ApiCallStats.Builder()
- .setCode(AD_SERVICES_API_CALLED)
- .setApiClass(AD_SERVICES_API_CALLED__API_CLASS__TARGETING)
- .setApiName(AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS)
- .setAppPackageName(mPackageName)
- .setSdkPackageName(mSdkName)
- .setLatencyMillisecond(LATENCY)
- .setResultCode(RESULT_OK)
- .build();
- Assert.assertEquals(AD_SERVICES_API_CALLED, stats.getCode());
- Assert.assertEquals(AD_SERVICES_API_CALLED__API_CLASS__TARGETING, stats.getApiClass());
- Assert.assertEquals(AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS, stats.getApiName());
- Assert.assertEquals(mPackageName, stats.getAppPackageName());
- Assert.assertEquals(mSdkName, stats.getSdkPackageName());
- Assert.assertEquals(LATENCY, stats.getLatencyMillisecond());
- Assert.assertEquals(RESULT_OK, stats.getResultCode());
+ ApiCallStats stats =
+ new ApiCallStats.Builder()
+ .setCode(AD_SERVICES_API_CALLED)
+ .setApiClass(AD_SERVICES_API_CALLED__API_CLASS__TARGETING)
+ .setApiName(AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS)
+ .setAppPackageName(mAppPackageName)
+ .setSdkPackageName(mSdkPackageName)
+ .setLatencyMillisecond(LATENCY_MS)
+ .setResultCode(STATUS_INVALID_ARGUMENT)
+ .build();
+
+ expect.withMessage("%s.getCode()", stats)
+ .that(stats.getCode())
+ .isEqualTo(AD_SERVICES_API_CALLED);
+ expect.withMessage("%s.getApiClass()", stats)
+ .that(stats.getApiClass())
+ .isEqualTo(AD_SERVICES_API_CALLED__API_CLASS__TARGETING);
+ expect.withMessage("%s.getApiName()", stats)
+ .that(stats.getApiName())
+ .isEqualTo(AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS);
+ expect.withMessage("%s.getAppPackageName()", stats)
+ .that(stats.getAppPackageName())
+ .isEqualTo(mAppPackageName);
+ expect.withMessage("%s.getSdkPackageName()", stats)
+ .that(stats.getSdkPackageName())
+ .isEqualTo(mSdkPackageName);
+ expect.withMessage("%s.getLatencyMillisecond()", stats)
+ .that(stats.getLatencyMillisecond())
+ .isEqualTo(LATENCY_MS);
+ expect.withMessage("%s.getResultCode()", stats)
+ .that(stats.getResultCode())
+ .isEqualTo(STATUS_INVALID_ARGUMENT);
}
@Test
public void testNullSdkPackageName_throwsNPE() {
- Assert.assertThrows("expect to throw null exception",
+ assertThrows(
NullPointerException.class,
- () -> {
- new ApiCallStats.Builder()
- .setSdkPackageName(null)
- .build();
- }
- );
+ () -> new ApiCallStats.Builder().setSdkPackageName(null).build());
}
@Test
public void testNullAppPackageName_throwsNPE() {
- Assert.assertThrows("expect to throw null exception",
+ assertThrows(
NullPointerException.class,
- () -> {
- new ApiCallStats.Builder()
- .setAppPackageName(null)
- .build();
- }
- );
+ () -> new ApiCallStats.Builder().setAppPackageName(null).build());
}
@Test
- public void testIncompleteInputBuilder_throwsIAE() {
- Assert.assertThrows("expect to throw Illegal Argument exception",
- IllegalArgumentException.class,
- () -> {
- new ApiCallStats.Builder()
- .build();
- }
- );
+ public void testBuild_noAppPackageName() {
+ assertThrows(
+ IllegalStateException.class,
+ () -> new ApiCallStats.Builder().setSdkPackageName("package.I.am").build());
+ }
+
+ @Test
+ public void testBuild_noSdkPackageName() {
+ assertThrows(
+ IllegalStateException.class,
+ () -> new ApiCallStats.Builder().setAppPackageName("package.I.am").build());
+ }
+
+ @Test
+ public void testToString() {
+ ApiCallStats stats =
+ new ApiCallStats.Builder()
+ .setCode(AD_SERVICES_API_CALLED)
+ .setApiClass(AD_SERVICES_API_CALLED__API_CLASS__TARGETING)
+ .setApiName(AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS)
+ .setAppPackageName(mAppPackageName)
+ .setSdkPackageName(mSdkPackageName)
+ .setLatencyMillisecond(LATENCY_MS)
+ .setResultCode(STATUS_INVALID_ARGUMENT)
+ .build();
+
+ String toString = stats.toString();
+
+ expect.that(toString).startsWith("ApiCallStats");
+ expect.that(toString).contains("Code=" + AD_SERVICES_API_CALLED);
+ expect.that(toString).contains("ApiClass=" + AD_SERVICES_API_CALLED__API_CLASS__TARGETING);
+ expect.that(toString).contains("ApiName=" + AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS);
+ expect.that(toString).contains("AppPackageName='" + mAppPackageName + "'");
+ expect.that(toString).contains("SdkPackageName='" + mSdkPackageName + "'");
+ expect.that(toString).contains("LatencyMillisecond=" + LATENCY_MS);
+ expect.that(toString).contains("ResultCode=" + STATUS_INVALID_ARGUMENT);
+ }
+
+ @Test
+ public void testEqualsHashCode() {
+ int baseCode = AD_SERVICES_API_CALLED;
+ int baseApiClass = AD_SERVICES_API_CALLED__API_CLASS__TARGETING;
+ int baseApiName = AD_SERVICES_API_CALLED__API_NAME__GET_TOPICS;
+ String baseAppPackageName = mAppPackageName;
+ String baseSdkPackageName = mSdkPackageName;
+ int baseLatencyMs = LATENCY_MS;
+ int resultCode = STATUS_INVALID_ARGUMENT;
+
+ ApiCallStats equals1 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats equals2 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different1 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode + 42)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different2 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass + 42)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different3 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName + 42)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different4 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName + ".doh")
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different5 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName + ".doh")
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different6 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs + 42)
+ .setResultCode(resultCode)
+ .build();
+
+ ApiCallStats different7 =
+ new ApiCallStats.Builder()
+ .setCode(baseCode)
+ .setApiClass(baseApiClass)
+ .setApiName(baseApiName)
+ .setAppPackageName(baseAppPackageName)
+ .setSdkPackageName(baseSdkPackageName)
+ .setLatencyMillisecond(baseLatencyMs)
+ .setResultCode(resultCode + 42)
+ .build();
+
+ expectObjectsAreEqual(equals1, equals2);
+
+ expectObjectsAreNotEqual(equals1, null);
+ expectObjectsAreNotEqual(equals1, "STATS, Y U NO STRING?");
+
+ expectObjectsAreNotEqual(equals1, different1);
+ expectObjectsAreNotEqual(equals1, different2);
+ expectObjectsAreNotEqual(equals1, different3);
+ expectObjectsAreNotEqual(equals1, different4);
+ expectObjectsAreNotEqual(equals1, different5);
+ expectObjectsAreNotEqual(equals1, different6);
+ expectObjectsAreNotEqual(equals1, different7);
}
}
