feed v2: populate upload action sequence number

We already have sequential numbers for actions, we
just weren't populating the proto yet.

Bug: b/155774256
Change-Id: Ie12b718079e44e9c8f2d702d5eb1e216c396db48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359375
Auto-Submit: Dan H <harringtond@chromium.org>
Commit-Queue: Ian Wells <iwells@chromium.org>
Reviewed-by: Ian Wells <iwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799146}

components/feed/core/v2/feed_stream.cc

@@ -122,6 +122,9 @@ void FeedStream::Metadata::SetConsistencyToken(std::string consistency_token) {
 
 LocalActionId FeedStream::Metadata::GetNextActionId() {
   uint32_t id = metadata_.next_action_id();
+  // Never use 0, as that's an invalid LocalActionId.
+  if (id == 0)
+    ++id;
   metadata_.set_next_action_id(id + 1);
   store_->WriteMetadata(metadata_, base::DoNothing());
   return LocalActionId(id);

components/feed/core/v2/feed_stream_unittest.cc

@@ -1569,6 +1569,31 @@ TEST_F(FeedStreamTest, LoadStreamFromNetworkUploadsActions) {
   EXPECT_EQ(1, network_.action_request_sent->feed_actions_size());
 }
 
+TEST_F(FeedStreamTest, UploadedActionsHaveSequentialNumbers) {
+  // Send 3 actions.
+  stream_->UploadAction(MakeFeedAction(1ul), false, base::DoNothing());
+  stream_->UploadAction(MakeFeedAction(2ul), false, base::DoNothing());
+  stream_->UploadAction(MakeFeedAction(3ul), true, base::DoNothing());
+  WaitForIdleTaskQueue();
+  ASSERT_EQ(1, network_.action_request_call_count);
+  feedwire::UploadActionsRequest request1 = *network_.action_request_sent;
+
+  // Send another action in a new request.
+  stream_->UploadAction(MakeFeedAction(4ul), true, base::DoNothing());
+  WaitForIdleTaskQueue();
+  ASSERT_EQ(2, network_.action_request_call_count);
+  feedwire::UploadActionsRequest request2 = *network_.action_request_sent;
+
+  // Verify that sent actions have sequential numbers.
+  ASSERT_EQ(3, request1.feed_actions_size());
+  ASSERT_EQ(1, request2.feed_actions_size());
+
+  EXPECT_EQ(1, request1.feed_actions(0).client_data().sequence_number());
+  EXPECT_EQ(2, request1.feed_actions(1).client_data().sequence_number());
+  EXPECT_EQ(3, request1.feed_actions(2).client_data().sequence_number());
+  EXPECT_EQ(4, request2.feed_actions(0).client_data().sequence_number());
+}
+
 TEST_F(FeedStreamTest, LoadMoreUploadsActions) {
   response_translator_.InjectResponse(MakeTypicalInitialModelState());
   TestSurface surface(stream_.get());
@@ -1711,14 +1736,14 @@ TEST_F(FeedStreamTest, MetadataLoadedWhenDatabaseInitialized) {
 
   // Set the token and increment next action ID.
   stream_->GetMetadata()->SetConsistencyToken("token");
-  EXPECT_EQ(0, stream_->GetMetadata()->GetNextActionId().GetUnsafeValue());
+  EXPECT_EQ(1, stream_->GetMetadata()->GetNextActionId().GetUnsafeValue());
 
   // Creating a stream should load metadata.
   CreateStream();
 
   ASSERT_TRUE(stream_->GetMetadata());
   EXPECT_EQ("token", stream_->GetMetadata()->GetConsistencyToken());
-  EXPECT_EQ(1, stream_->GetMetadata()->GetNextActionId().GetUnsafeValue());
+  EXPECT_EQ(2, stream_->GetMetadata()->GetNextActionId().GetUnsafeValue());
 }
 
 TEST_F(FeedStreamTest, ModelUnloadsAfterTimeout) {

components/feed/core/v2/tasks/upload_actions_task.cc

@@ -139,7 +139,10 @@ void UploadActionsTask::Run() {
   // to upload all pending actions.
   if (wire_action_) {
     StoredAction action;
-    action.set_id(stream_->GetMetadata()->GetNextActionId().GetUnsafeValue());
+    int32_t action_id =
+        stream_->GetMetadata()->GetNextActionId().GetUnsafeValue();
+    action.set_id(action_id);
+    wire_action_->mutable_client_data()->set_sequence_number(action_id);
     *action.mutable_action() = std::move(*wire_action_);
     // No need to set upload_attempt_count as it defaults to 0.
     // WriteActions() sets the ID.