[Sync] Add tests for invalid specifics field number handling

sync/engine/syncer_proto_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sync/engine/syncer_proto_util.h"
 
 #include "base/format_macros.h"
 #include "base/stringprintf.h"
 #include "google_apis/google_api_keys.h"
 #include "sync/engine/net/server_connection_manager.h"
 #include "sync/engine/syncer.h"
 #include "sync/engine/syncer_types.h"
 #include "sync/engine/throttled_data_type_tracker.h"
 #include "sync/engine/traffic_logger.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/protocol/sync.pb.h"
 #include "sync/protocol/sync_enums.pb.h"
 #include "sync/protocol/sync_protocol_error.h"
 #include "sync/sessions/sync_session.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/syncable-inl.h"
 #include "sync/syncable/syncable_proto_util.h"
 #include "sync/util/time.h"
 
 using std::string;
@@ skipping 73 matching lines @@
     case HttpResponse::RETRY:
       return SERVER_RETURN_TRANSIENT_ERROR;
     case HttpResponse::SERVER_CONNECTION_OK:
     case HttpResponse::NONE:
     default:
       NOTREACHED();
       return UNSET;
   }
 }
 
+SyncProtocolErrorType ConvertSyncProtocolErrorTypePBToLocalType(
+    const sync_pb::SyncEnums::ErrorType& error_type) {
+  switch (error_type) {
+    case sync_pb::SyncEnums::SUCCESS:
+      return SYNC_SUCCESS;
+    case sync_pb::SyncEnums::NOT_MY_BIRTHDAY:
+      return NOT_MY_BIRTHDAY;
+    case sync_pb::SyncEnums::THROTTLED:
+      return THROTTLED;
+    case sync_pb::SyncEnums::CLEAR_PENDING:
+      return CLEAR_PENDING;
+    case sync_pb::SyncEnums::TRANSIENT_ERROR:
+      return TRANSIENT_ERROR;
+    case sync_pb::SyncEnums::MIGRATION_DONE:
+      return MIGRATION_DONE;
+    case sync_pb::SyncEnums::UNKNOWN:
+      return UNKNOWN_ERROR;
+    case sync_pb::SyncEnums::USER_NOT_ACTIVATED:
+    case sync_pb::SyncEnums::AUTH_INVALID:
+    case sync_pb::SyncEnums::ACCESS_DENIED:
+      return INVALID_CREDENTIAL;
+    default:
+      NOTREACHED();
+      return UNKNOWN_ERROR;
+  }
+}
+
+ClientAction ConvertClientActionPBToLocalClientAction(
+    const sync_pb::SyncEnums::Action& action) {
+  switch (action) {
+    case sync_pb::SyncEnums::UPGRADE_CLIENT:
+      return UPGRADE_CLIENT;
+    case sync_pb::SyncEnums::CLEAR_USER_DATA_AND_RESYNC:
+      return CLEAR_USER_DATA_AND_RESYNC;
+    case sync_pb::SyncEnums::ENABLE_SYNC_ON_ACCOUNT:
+      return ENABLE_SYNC_ON_ACCOUNT;
+    case sync_pb::SyncEnums::STOP_AND_RESTART_SYNC:
+      return STOP_AND_RESTART_SYNC;
+    case sync_pb::SyncEnums::DISABLE_SYNC_ON_CLIENT:
+      return DISABLE_SYNC_ON_CLIENT;
+    case sync_pb::SyncEnums::UNKNOWN_ACTION:
+      return UNKNOWN_ACTION;
+    default:
+      NOTREACHED();
+      return UNKNOWN_ACTION;
+  }
+}
+
 }  // namespace
 
-// static
-void SyncerProtoUtil::HandleMigrationDoneResponse(
-  const ClientToServerResponse* response,
-  sessions::SyncSession* session) {
-  LOG_IF(ERROR, 0 >= response->migrated_data_type_id_size())
-      << "MIGRATION_DONE but no types specified.";
+ModelTypeSet GetTypesToMigrate(const ClientToServerResponse& response) {
   ModelTypeSet to_migrate;
-  for (int i = 0; i < response->migrated_data_type_id_size(); i++) {
-    int field_number = response->migrated_data_type_id(i);
+  for (int i = 0; i < response.migrated_data_type_id_size(); i++) {
+    int field_number = response.migrated_data_type_id(i);
     ModelType model_type = GetModelTypeFromSpecificsFieldNumber(field_number);
     if (!IsRealDataType(model_type)) {
-      NOTREACHED() << "Unknown field number " << field_number;
+      DLOG(WARNING) << "Unknown field number " << field_number;
       continue;
     }
     to_migrate.Put(model_type);
   }
-  // TODO(akalin): This should be a set union.
-  session->mutable_status_controller()->
-      set_types_needing_local_migration(to_migrate);
+  return to_migrate;
 }
 
-// static
-bool SyncerProtoUtil::VerifyResponseBirthday(syncable::Directory* dir,
-    const ClientToServerResponse* response) {
+SyncProtocolError ConvertErrorPBToLocalType(
+    const sync_pb::ClientToServerResponse_Error& error) {
+  SyncProtocolError sync_protocol_error;
+  sync_protocol_error.error_type = ConvertSyncProtocolErrorTypePBToLocalType(
+      error.error_type());
+  sync_protocol_error.error_description = error.error_description();
+  sync_protocol_error.url = error.url();
+  sync_protocol_error.action = ConvertClientActionPBToLocalClientAction(
+      error.action());
+
+  if (error.error_data_type_ids_size() > 0) {
+    // THROTTLED is currently the only error code that uses |error_data_types|.
+    DCHECK_EQ(error.error_type(), sync_pb::SyncEnums::THROTTLED);
+    for (int i = 0; i < error.error_data_type_ids_size(); ++i) {
+      int field_number = error.error_data_type_ids(i);
+      ModelType model_type =
+          GetModelTypeFromSpecificsFieldNumber(field_number);
+      if (!IsRealDataType(model_type)) {
+        DLOG(WARNING) << "Unknown field number " << field_number;
+        continue;
+      }
+      sync_protocol_error.error_data_types.Put(model_type);
+    }
+  }
+
+  return sync_protocol_error;
+}
+
+bool SyncerProtoUtil::VerifyResponseBirthday(
+    const ClientToServerResponse& response,
+    syncable::Directory* dir) {
 
   std::string local_birthday = dir->store_birthday();
 
   if (local_birthday.empty()) {
-    if (!response->has_store_birthday()) {
+    if (!response.has_store_birthday()) {
       LOG(WARNING) << "Expected a birthday on first sync.";
       return false;
     }
 
-    DVLOG(1) << "New store birthday: " << response->store_birthday();
-    dir->set_store_birthday(response->store_birthday());
+    DVLOG(1) << "New store birthday: " << response.store_birthday();
+    dir->set_store_birthday(response.store_birthday());
     return true;
   }
 
   // Error situation, but we're not stuck.
-  if (!response->has_store_birthday()) {
+  if (!response.has_store_birthday()) {
     LOG(WARNING) << "No birthday in server response?";
     return true;
   }
 
-  if (response->store_birthday() != local_birthday) {
+  if (response.store_birthday() != local_birthday) {
     LOG(WARNING) << "Birthday changed, showing syncer stuck";
     return false;
   }
 
   return true;
 }
 
 // static
 void SyncerProtoUtil::AddRequestBirthday(syncable::Directory* dir,
                                          ClientToServerMessage* msg) {
@@ skipping 91 matching lines @@
   if (!message.has_get_updates())
     return false;
   DCHECK_LT(0, message.get_updates().from_progress_marker_size());
   for (int i = 0; i < message.get_updates().from_progress_marker_size(); ++i) {
     if (!message.get_updates().from_progress_marker(i).token().empty())
       return false;
   }
   return true;
 }
 
-SyncProtocolErrorType ConvertSyncProtocolErrorTypePBToLocalType(
-    const sync_pb::SyncEnums::ErrorType& error_type) {
-  switch (error_type) {
-    case sync_pb::SyncEnums::SUCCESS:
-      return SYNC_SUCCESS;
-    case sync_pb::SyncEnums::NOT_MY_BIRTHDAY:
-      return NOT_MY_BIRTHDAY;
-    case sync_pb::SyncEnums::THROTTLED:
-      return THROTTLED;
-    case sync_pb::SyncEnums::CLEAR_PENDING:
-      return CLEAR_PENDING;
-    case sync_pb::SyncEnums::TRANSIENT_ERROR:
-      return TRANSIENT_ERROR;
-    case sync_pb::SyncEnums::MIGRATION_DONE:
-      return MIGRATION_DONE;
-    case sync_pb::SyncEnums::UNKNOWN:
-      return UNKNOWN_ERROR;
-    case sync_pb::SyncEnums::USER_NOT_ACTIVATED:
-    case sync_pb::SyncEnums::AUTH_INVALID:
-    case sync_pb::SyncEnums::ACCESS_DENIED:
-      return INVALID_CREDENTIAL;
-    default:
-      NOTREACHED();
-      return UNKNOWN_ERROR;
-  }
-}
-
-ClientAction ConvertClientActionPBToLocalClientAction(
-    const sync_pb::SyncEnums::Action& action) {
-  switch (action) {
-    case sync_pb::SyncEnums::UPGRADE_CLIENT:
-      return UPGRADE_CLIENT;
-    case sync_pb::SyncEnums::CLEAR_USER_DATA_AND_RESYNC:
-      return CLEAR_USER_DATA_AND_RESYNC;
-    case sync_pb::SyncEnums::ENABLE_SYNC_ON_ACCOUNT:
-      return ENABLE_SYNC_ON_ACCOUNT;
-    case sync_pb::SyncEnums::STOP_AND_RESTART_SYNC:
-      return STOP_AND_RESTART_SYNC;
-    case sync_pb::SyncEnums::DISABLE_SYNC_ON_CLIENT:
-      return DISABLE_SYNC_ON_CLIENT;
-    case sync_pb::SyncEnums::UNKNOWN_ACTION:
-      return UNKNOWN_ACTION;
-    default:
-      NOTREACHED();
-      return UNKNOWN_ACTION;
-  }
-}
-
-SyncProtocolError ConvertErrorPBToLocalType(
-    const ClientToServerResponse::Error& error) {
-  SyncProtocolError sync_protocol_error;
-  sync_protocol_error.error_type = ConvertSyncProtocolErrorTypePBToLocalType(
-      error.error_type());
-  sync_protocol_error.error_description = error.error_description();
-  sync_protocol_error.url = error.url();
-  sync_protocol_error.action = ConvertClientActionPBToLocalClientAction(
-      error.action());
-
-  if (error.error_data_type_ids_size() > 0) {
-    // THROTTLED is currently the only error code that uses |error_data_types|.
-    DCHECK_EQ(error.error_type(), sync_pb::SyncEnums::THROTTLED);
-    for (int i = 0; i < error.error_data_type_ids_size(); ++i) {
-      int field_number = error.error_data_type_ids(i);
-      ModelType model_type =
-          GetModelTypeFromSpecificsFieldNumber(field_number);
-      if (!IsRealDataType(model_type)) {
-        NOTREACHED() << "Unknown field number " << field_number;
-        continue;
-      }
-      sync_protocol_error.error_data_types.Put(model_type);
-    }
-  }
-
-  return sync_protocol_error;
-}
-
 // TODO(lipalani) : Rename these function names as per the CR for issue 7740067.
 SyncProtocolError ConvertLegacyErrorCodeToNewError(
     const sync_pb::SyncEnums::ErrorType& error_type) {
   SyncProtocolError error;
   error.error_type = ConvertSyncProtocolErrorTypePBToLocalType(error_type);
   if (error_type == sync_pb::SyncEnums::CLEAR_PENDING ||
       error_type == sync_pb::SyncEnums::NOT_MY_BIRTHDAY) {
       error.action = DISABLE_SYNC_ON_CLIENT;
   }  // There is no other action we can compute for legacy server.
   return error;
 }
 
 }  // namespace
 
 // static
 SyncerError SyncerProtoUtil::PostClientToServerMessage(
     ClientToServerMessage* msg,
     ClientToServerResponse* response,
     SyncSession* session) {
-
   CHECK(response);
   DCHECK(!msg->get_updates().has_from_timestamp());  // Deprecated.
   DCHECK(!msg->get_updates().has_requested_types());  // Deprecated.
 
   // Add must-have fields.
   SetProtocolVersion(msg);
   AddRequestBirthday(session->context()->directory(), msg);
   DCHECK(msg->has_store_birthday() || IsVeryFirstGetUpdates(*msg));
   AddBagOfChips(session->context()->directory(), msg);
   msg->set_api_key(google_apis::GetAPIKey());
@@ skipping 19 matching lines @@
   LogClientToServerResponse(*response);
   session->context()->traffic_recorder()->RecordClientToServerResponse(
       *response);
 
   // Persist a bag of chips if it has been sent by the server.
   PersistBagOfChips(dir, *response);
 
   SyncProtocolError sync_protocol_error;
 
   // Birthday mismatch overrides any error that is sent by the server.
-  if (!VerifyResponseBirthday(dir, response)) {
+  if (!VerifyResponseBirthday(*response, dir)) {
     sync_protocol_error.error_type = NOT_MY_BIRTHDAY;
      sync_protocol_error.action =
          DISABLE_SYNC_ON_CLIENT;
   } else if (response->has_error()) {
     // This is a new server. Just get the error from the protocol.
     sync_protocol_error = ConvertErrorPBToLocalType(response->error());
   } else {
     // Legacy server implementation. Compute the error based on |error_code|.
     sync_protocol_error = ConvertLegacyErrorCodeToNewError(
         response->error_code());
@@ skipping 44 matching lines @@
     case THROTTLED:
       LOG(WARNING) << "Client silenced by server.";
       HandleThrottleError(sync_protocol_error,
                           base::TimeTicks::Now() + GetThrottleDelay(*response),
                           session->context()->throttled_data_type_tracker(),
                           session->delegate());
       return SERVER_RETURN_THROTTLED;
     case TRANSIENT_ERROR:
       return SERVER_RETURN_TRANSIENT_ERROR;
     case MIGRATION_DONE:
-      HandleMigrationDoneResponse(response, session);
+      LOG_IF(ERROR, 0 >= response->migrated_data_type_id_size())
+          << "MIGRATION_DONE but no types specified.";
+      // TODO(akalin): This should be a set union.
+      session->mutable_status_controller()->
+          set_types_needing_local_migration(GetTypesToMigrate(*response));
       return SERVER_RETURN_MIGRATION_DONE;
     case CLEAR_PENDING:
       return SERVER_RETURN_CLEAR_PENDING;
     case NOT_MY_BIRTHDAY:
       return SERVER_RETURN_NOT_MY_BIRTHDAY;
     default:
       NOTREACHED();
       return UNSET;
   }
 }
@@ skipping 129 matching lines @@
 std::string SyncerProtoUtil::ClientToServerResponseDebugString(
     const ClientToServerResponse& response) {
   // Add more handlers as needed.
   std::string output;
   if (response.has_get_updates())
     output.append(GetUpdatesResponseString(response.get_updates()));
   return output;
 }
 
 }  // namespace syncer