Guarantee an increasing sequence in the onVmsSubscriptionChange message

First, instead of passing a sequence number and a list of layers, wrap
them in a parcelable object.

Then, generate the sequence number when updating the set of
subscriptions. In this way, we guarantee that different snapshots of
the subscription data have different sequence numbers.

Test: runtest -x packages/services/Car/tests/carservice_test/src/com/android/car/test/VmsPublisherClientServiceTest.java
Test: runtest -x packages/services/Car/tests/carservice_test/src/com/android/car/test/VmsPublisherSubscriberTest.java
Test: runtest -x packages/services/Car/tests/carservice_test/src/com/android/car/test/VmsSubscriberManagerTest.java
Test: runtest -x packages/services/Car/tests/carservice_unit_test/src/com/android/car/VmsRoutingTest.java
Bug: 36585514
Change-Id: I65cfd2aa1b0c1085bac13a3b77d472ff479da357

5 files changed, 128 insertions, 43 deletions

diff --git a/car-lib/src/android/car/vms/IVmsPublisherClient.aidl b/car-lib/src/android/car/vms/IVmsPublisherClient.aidl
index f454a33576..96b993b3f4 100644
--- a/car-lib/src/android/car/vms/IVmsPublisherClient.aidl
+++ b/car-lib/src/android/car/vms/IVmsPublisherClient.aidl
@@ -17,7 +17,7 @@
 package android.car.vms;
 
 import android.car.vms.IVmsPublisherService;
-import android.car.vms.VmsLayer;
+import android.car.vms.VmsSubscriptionState;
 
 /**
  * @hide
@@ -32,10 +32,9 @@ interface IVmsPublisherClient {
 
     /**
      * The VmsPublisherService uses this callback to notify about subscription changes.
-     * @param layers   all the layers that have subscribers.
-     * @param sequence a monotonicallly increasing number, clients should ignore any packet with a
-     *                 sequence number that is less than the highest sequence number they have seen
-     *                 thus far.
+     * @param subscriptionState all the layers that have subscribers and a sequence number,
+     *                          clients should ignore any packet with a sequence number that is less
+     *                          than the highest sequence number they have seen thus far.
      */
-    oneway void onVmsSubscriptionChange(in List<VmsLayer> layers, long sequence) = 1;
+    oneway void onVmsSubscriptionChange(in VmsSubscriptionState subscriptionState) = 1;
 }
diff --git a/car-lib/src/android/car/vms/IVmsPublisherService.aidl b/car-lib/src/android/car/vms/IVmsPublisherService.aidl
index e7464579cd..26b6e523ed 100644
--- a/car-lib/src/android/car/vms/IVmsPublisherService.aidl
+++ b/car-lib/src/android/car/vms/IVmsPublisherService.aidl
@@ -18,6 +18,7 @@ package android.car.vms;
 
 import android.car.vms.VmsLayer;
 import android.car.vms.VmsLayersOffering;
+import android.car.vms.VmsSubscriptionState;
 
 /**
  * Exposes publisher services to VMS clients.
@@ -33,7 +34,7 @@ interface IVmsPublisherService {
     /**
      * Returns the list of VmsLayers that has any clients subscribed to it.
      */
-    List<VmsLayer> getSubscriptions() = 1;
+    VmsSubscriptionState getSubscriptions() = 1;
 
     /**
      * Sets which layers the publisher can publish under which dependencties.
diff --git a/car-lib/src/android/car/vms/VmsPublisherClientService.java b/car-lib/src/android/car/vms/VmsPublisherClientService.java
index 5fae38c687..2743ff157b 100644
--- a/car-lib/src/android/car/vms/VmsPublisherClientService.java
+++ b/car-lib/src/android/car/vms/VmsPublisherClientService.java
@@ -90,13 +90,10 @@ public abstract class VmsPublisherClientService extends Service {
 
     /**
      * Publishers need to implement this method to receive notifications of subscription changes.
-     * TODO(antoniocortes): evaluate adding subscriber id to the list of parameters. This should be
-     * implemented together with the routing in VmsPublisherService.
      *
-     * @param layers          layers with subscribers.
-     * @param sequence        monotonically increasing sequence.
+     * @param subscriptionState  layers with subscribers and a sequence number.
      */
-    public abstract void onVmsSubscriptionChange(List<VmsLayer> layers, long sequence);
+    public abstract void onVmsSubscriptionChange(VmsSubscriptionState subscriptionState);
 
     /**
      * Uses the VmsPublisherService binder to publish messages.
@@ -135,7 +132,7 @@ public abstract class VmsPublisherClientService extends Service {
      *
      * @return list of layer/version or null in case of error.
      */
-    public final @Nullable List<VmsLayer> getSubscriptions() {
+    public final @Nullable VmsSubscriptionState getSubscriptions() {
         if (mVmsPublisherService == null) {
             throw new IllegalStateException("VmsPublisherService not set.");
         }
@@ -180,28 +177,27 @@ public abstract class VmsPublisherClientService extends Service {
         }
 
         @Override
-        public void onVmsSubscriptionChange(List<VmsLayer> layers, long sequence)
+        public void onVmsSubscriptionChange(VmsSubscriptionState subscriptionState)
                 throws RemoteException {
             VmsPublisherClientService vmsPublisherClientService = mVmsPublisherClientService.get();
             if (vmsPublisherClientService == null) return;
             if (DBG) {
-                Log.d(TAG, "subscription event, # layers: " + layers.size()
-                        + ", sequence: " + sequence);
+                Log.d(TAG, "subscription event: " + subscriptionState);
             }
             synchronized (mSequenceLock) {
-                if (sequence <= mSequence) {
+                if (subscriptionState.getSequenceNumber() <= mSequence) {
                     Log.w(TAG, "Sequence out of order. Current sequence = " + mSequence
-                            + "; expected new sequence = " + sequence);
+                            + "; expected new sequence = " + subscriptionState.getSequenceNumber());
                     // Do not propagate old notifications.
                     return;
                 } else {
-                    mSequence = sequence;
+                    mSequence = subscriptionState.getSequenceNumber();
                 }
             }
             Handler handler = vmsPublisherClientService.mHandler;
             handler.sendMessage(
                     handler.obtainMessage(VmsEventHandler.ON_SUBSCRIPTION_CHANGE_EVENT,
-                            new OnVmsSubscriptionChangeData(layers, sequence)));
+                            subscriptionState));
         }
     }
 
@@ -226,8 +222,8 @@ public abstract class VmsPublisherClientService extends Service {
             if (service == null) return;
             switch (msg.what) {
                 case ON_SUBSCRIPTION_CHANGE_EVENT:
-                    OnVmsSubscriptionChangeData data = (OnVmsSubscriptionChangeData) msg.obj;
-                    service.onVmsSubscriptionChange(data.getLayers(), data.getSequence());
+                    VmsSubscriptionState subscriptionState = (VmsSubscriptionState) msg.obj;
+                    service.onVmsSubscriptionChange(subscriptionState);
                     break;
                 case SET_SERVICE_CALLBACK:
                     service.setVmsPublisherService((IVmsPublisherService) msg.obj);
@@ -238,25 +234,4 @@ public abstract class VmsPublisherClientService extends Service {
             }
         }
     }
-
-    /**
-     * Used to forward data from the binder thread to the main thread.
-     */
-    private static final class OnVmsSubscriptionChangeData {
-        private final List<VmsLayer> mLayers;
-        private final long mSequence;
-
-        public OnVmsSubscriptionChangeData(List<VmsLayer> layers, long sequence) {
-            mLayers = layers;
-            mSequence = sequence;
-        }
-
-        public List<VmsLayer> getLayers() {
-            return mLayers;
-        }
-
-        public long getSequence() {
-            return mSequence;
-        }
-    }
 }
diff --git a/car-lib/src/android/car/vms/VmsSubscriptionState.aidl b/car-lib/src/android/car/vms/VmsSubscriptionState.aidl
new file mode 100644
index 0000000000..b5ce8fff19
--- /dev/null
+++ b/car-lib/src/android/car/vms/VmsSubscriptionState.aidl
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2017 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.car.vms;
+
+parcelable VmsSubscriptionState;
\ No newline at end of file
diff --git a/car-lib/src/android/car/vms/VmsSubscriptionState.java b/car-lib/src/android/car/vms/VmsSubscriptionState.java
new file mode 100644
index 0000000000..0e36fb1f30
--- /dev/null
+++ b/car-lib/src/android/car/vms/VmsSubscriptionState.java
@@ -0,0 +1,91 @@
+/*
+ * Copyright (C) 2017 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.car.vms;
+
+import android.car.annotation.FutureFeature;
+import android.os.Parcel;
+import android.os.Parcelable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * The list of layers with subscribers.
+ *
+ * @hide
+ */
+@FutureFeature
+public final class VmsSubscriptionState implements Parcelable {
+    private final int mSequenceNumber;
+    private final List<VmsLayer> mLayers;
+
+    /**
+     * Construct a dependency for layer on other layers.
+     */
+    public VmsSubscriptionState(int sequenceNumber, List<VmsLayer> dependencies) {
+        mSequenceNumber = sequenceNumber;
+        mLayers = Collections.unmodifiableList(dependencies);
+    }
+
+    public int getSequenceNumber() {
+        return mSequenceNumber;
+    }
+
+    public List<VmsLayer> getLayers() {
+        return mLayers;
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("sequence number=").append(mSequenceNumber);
+        sb.append("; layers={");
+        for(VmsLayer layer : mLayers) {
+            sb.append(layer).append(",");
+        }
+        sb.append("}");
+        return sb.toString();
+    }
+
+    public static final Parcelable.Creator<VmsSubscriptionState> CREATOR = new
+        Parcelable.Creator<VmsSubscriptionState>() {
+            public VmsSubscriptionState createFromParcel(Parcel in) {
+                return new VmsSubscriptionState(in);
+            }
+            public VmsSubscriptionState[] newArray(int size) {
+                return new VmsSubscriptionState[size];
+            }
+        };
+
+    @Override
+    public void writeToParcel(Parcel out, int flags) {
+        out.writeInt(mSequenceNumber);
+        out.writeParcelableList(mLayers, flags);
+    }
+
+    @Override
+    public int describeContents() {
+        return 0;
+    }
+
+    private VmsSubscriptionState(Parcel in) {
+        mSequenceNumber = in.readInt();
+        List<VmsLayer> layers = new ArrayList<>();
+        in.readParcelableList(layers, VmsLayer.class.getClassLoader());
+        mLayers = Collections.unmodifiableList(layers);
+    }
+}
\ No newline at end of file