Merge "[CarTelemetryService] implement sendFinishReports" into sc-v2-dev
diff --git a/service/src/com/android/car/telemetry/CarTelemetryService.java b/service/src/com/android/car/telemetry/CarTelemetryService.java
index 8e694d0..16d3509 100644
--- a/service/src/com/android/car/telemetry/CarTelemetryService.java
+++ b/service/src/com/android/car/telemetry/CarTelemetryService.java
@@ -29,6 +29,7 @@
 import android.content.SharedPreferences;
 import android.os.Handler;
 import android.os.HandlerThread;
+import android.os.PersistableBundle;
 import android.os.RemoteException;
 import android.util.IndentingPrintWriter;
 import android.util.Slog;
@@ -50,7 +51,9 @@
 
 import com.google.protobuf.InvalidProtocolBufferException;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.IOException;
 import java.util.List;
 
 /**
@@ -153,7 +156,8 @@
     }
 
     /**
-     * Send a telemetry metrics config to the service.
+     * Send a telemetry metrics config to the service. This method assumes
+     * {@link #setListener(ICarTelemetryServiceListener)} is called. Otherwise it does nothing.
      *
      * @param key    the unique key to identify the MetricsConfig.
      * @param config the serialized bytes of a MetricsConfig object.
@@ -163,9 +167,12 @@
         mContext.enforceCallingOrSelfPermission(
                 Car.PERMISSION_USE_CAR_TELEMETRY_SERVICE, "addMetricsConfig");
         mTelemetryHandler.post(() -> {
+            if (mListener == null) {
+                Slog.w(CarLog.TAG_TELEMETRY, "ICarTelemetryServiceListener is not set");
+                return;
+            }
             Slog.d(CarLog.TAG_TELEMETRY, "Adding metrics config " + key.getName()
                     + " to car telemetry service");
-            // TODO(b/199540952): If config is active, add it to DataBroker
             TelemetryProto.MetricsConfig metricsConfig = null;
             int status = ERROR_METRICS_CONFIG_UNKNOWN;
             try {
@@ -177,6 +184,8 @@
             // if config can be parsed, add it to persistent storage
             if (metricsConfig != null) {
                 status = mMetricsConfigStore.addMetricsConfig(metricsConfig);
+                // TODO(b/199410900): update logic once metrics configs have expiration dates
+                mDataBroker.addMetricsConfiguration(metricsConfig);
             }
             // If no error (a config is successfully added), script results from an older version
             // should be deleted
@@ -186,20 +195,25 @@
             try {
                 mListener.onAddMetricsConfigStatus(key, status);
             } catch (RemoteException e) {
-                Slog.d(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
+                Slog.w(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
             }
         });
     }
 
     /**
      * Removes a metrics config based on the key. This will also remove outputs produced by the
-     * MetricsConfig.
+     * MetricsConfig. This method assumes {@link #setListener(ICarTelemetryServiceListener)} is
+     * called. Otherwise it does nothing.
+     *
+     * @param key the unique identifier of a MetricsConfig.
      */
     @Override
     public void removeMetricsConfig(@NonNull MetricsConfigKey key) {
-        mContext.enforceCallingOrSelfPermission(
-                Car.PERMISSION_USE_CAR_TELEMETRY_SERVICE, "removeMetricsConfig");
         mTelemetryHandler.post(() -> {
+            if (mListener == null) {
+                Slog.w(CarLog.TAG_TELEMETRY, "ICarTelemetryServiceListener is not set");
+                return;
+            }
             Slog.d(CarLog.TAG_TELEMETRY, "Removing metrics config " + key.getName()
                     + " from car telemetry service");
             // TODO(b/198792767): Check both config name and config version for deletion
@@ -209,7 +223,7 @@
             try {
                 mListener.onRemoveMetricsConfigStatus(key, success);
             } catch (RemoteException e) {
-                Slog.d(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
+                Slog.w(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
             }
         });
     }
@@ -232,16 +246,35 @@
 
     /**
      * Sends script results associated with the given key using the
-     * {@link ICarTelemetryServiceListener}.
+     * {@link ICarTelemetryServiceListener}. This method assumes listener is set. Otherwise it
+     * does nothing.
+     *
+     * @param key the unique identifier of a MetricsConfig.
      */
     @Override
     public void sendFinishedReports(@NonNull MetricsConfigKey key) {
-        // TODO(b/184087869): Implement
         mContext.enforceCallingOrSelfPermission(
                 Car.PERMISSION_USE_CAR_TELEMETRY_SERVICE, "sendFinishedReports");
-        if (DEBUG) {
-            Slog.d(CarLog.TAG_TELEMETRY, "Flushing reports for a metrics config");
-        }
+        mTelemetryHandler.post(() -> {
+            if (mListener == null) {
+                Slog.w(CarLog.TAG_TELEMETRY, "ICarTelemetryServiceListener is not set");
+                return;
+            }
+            if (DEBUG) {
+                Slog.d(CarLog.TAG_TELEMETRY,
+                        "Flushing reports for metrics config " + key.getName());
+            }
+            PersistableBundle result = mResultStore.getFinalResult(key.getName(), true);
+            TelemetryProto.TelemetryError error = mResultStore.getError(key.getName(), true);
+            if (result != null) {
+                sendFinalResult(key, result);
+            } else if (error != null) {
+                sendError(key, error);
+            } else {
+                Slog.w(CarLog.TAG_TELEMETRY, "config " + key.getName()
+                        + " did not produce any results");
+            }
+        });
     }
 
     /**
@@ -257,6 +290,25 @@
         }
     }
 
+    private void sendFinalResult(MetricsConfigKey key, PersistableBundle result) {
+        try (ByteArrayOutputStream bos = new ByteArrayOutputStream()) {
+            result.writeToStream(bos);
+            mListener.onResult(key, bos.toByteArray());
+        } catch (RemoteException e) {
+            Slog.w(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
+        } catch (IOException e) {
+            Slog.w(CarLog.TAG_TELEMETRY, "failed to write bundle to output stream", e);
+        }
+    }
+
+    private void sendError(MetricsConfigKey key, TelemetryProto.TelemetryError error) {
+        try {
+            mListener.onError(key, error.toByteArray());
+        } catch (RemoteException e) {
+            Slog.w(CarLog.TAG_TELEMETRY, "error with ICarTelemetryServiceListener", e);
+        }
+    }
+
     @VisibleForTesting
     Handler getTelemetryHandler() {
         return mTelemetryHandler;
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java
index 39389d2..f65d32e 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/CarTelemetryServiceTest.java
@@ -19,7 +19,9 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -45,6 +47,7 @@
 import org.mockito.junit.MockitoJUnitRunner;
 import org.mockito.junit.MockitoRule;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.nio.file.Files;
 import java.util.concurrent.CountDownLatch;
@@ -206,6 +209,62 @@
         assertThat(mResultStore.getFinalResult(key.getName(), /* deleteResult = */ false)).isNull();
     }
 
+    @Test
+    public void testSendFinishedReports_whenNoReport_shouldNotReceiveResponse() throws Exception {
+        mService.sendFinishedReports(KEY_V1);
+
+        waitForHandlerThreadToFinish();
+        verify(mMockListener, never()).onResult(any(), any());
+        verify(mMockListener, never()).onError(any(), any());
+    }
+
+    @Test
+    public void testSendFinishedReports_whenFinalResult_shouldReceiveResult() throws Exception {
+        PersistableBundle finalResult = new PersistableBundle();
+        finalResult.putBoolean("finished", true);
+        mResultStore.putFinalResult(KEY_V1.getName(), finalResult);
+
+        mService.sendFinishedReports(KEY_V1);
+
+        waitForHandlerThreadToFinish();
+        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        finalResult.writeToStream(bos);
+        verify(mMockListener).onResult(eq(KEY_V1), eq(bos.toByteArray()));
+        // result should have been deleted
+        assertThat(mResultStore.getFinalResult(KEY_V1.getName(), false)).isNull();
+    }
+
+    @Test
+    public void testSendFinishedReports_whenError_shouldReceiveError() throws Exception {
+        TelemetryProto.TelemetryError error = TelemetryProto.TelemetryError.newBuilder()
+                .setErrorType(TelemetryProto.TelemetryError.ErrorType.LUA_RUNTIME_ERROR)
+                .setMessage("test error")
+                .build();
+        mResultStore.putError(KEY_V1.getName(), error);
+
+        mService.sendFinishedReports(KEY_V1);
+
+        waitForHandlerThreadToFinish();
+        verify(mMockListener).onError(eq(KEY_V1), eq(error.toByteArray()));
+        // error should have been deleted
+        assertThat(mResultStore.getError(KEY_V1.getName(), false)).isNull();
+    }
+
+    @Test
+    public void testSendFinishedReports_whenListenerNotSet_shouldDoNothing() throws Exception {
+        PersistableBundle finalResult = new PersistableBundle();
+        finalResult.putBoolean("finished", true);
+        mResultStore.putFinalResult(KEY_V1.getName(), finalResult);
+        mService.clearListener(); // no listener = no way to send back results
+
+        mService.sendFinishedReports(KEY_V1);
+
+        waitForHandlerThreadToFinish();
+        // if listener is null, nothing should be done, result should still be in result store
+        assertThat(mResultStore.getFinalResult(KEY_V1.getName(), false).toString())
+                .isEqualTo(finalResult.toString());
+    }
+
     private void waitForHandlerThreadToFinish() throws Exception {
         assertWithMessage("handler not idle in %sms", TIMEOUT_MS)
                 .that(mIdleHandlerLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)).isTrue();