Process TeamDrive change in change list.

components/drive/chromeos/change_list_processor.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 "components/drive/chromeos/change_list_processor.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/synchronization/cancellation_flag.h"
 #include "components/drive/chromeos/resource_metadata.h"
 #include "components/drive/drive.pb.h"
 #include "components/drive/drive_api_util.h"
 #include "components/drive/file_change.h"
 #include "components/drive/file_system_core_util.h"
 #include "components/drive/resource_entry_conversion.h"
 #include "google_apis/drive/drive_api_parser.h"
+#include "google_apis/drive/drive_switches.h"
 
 namespace drive {
 namespace internal {
 
 namespace {
 
 class ChangeListToEntryMapUMAStats {
  public:
   ChangeListToEntryMapUMAStats()
     : num_regular_files_(0),
@@ skipping 37 matching lines @@
 }
 
 ChangeList::ChangeList() {}
 
 ChangeList::ChangeList(const google_apis::ChangeList& change_list)
     : next_url_(change_list.next_link()),
       largest_changestamp_(change_list.largest_change_id()) {
   const std::vector<std::unique_ptr<google_apis::ChangeResource>>& items =
       change_list.items();
   entries_.resize(items.size());
+  team_drives_.resize(items.size());
   parent_resource_ids_.resize(items.size());
   size_t entries_index = 0;
+  size_t team_drives_index = 0;
   for (size_t i = 0; i < items.size(); ++i) {
-    if (ConvertChangeResourceToResourceEntry(
-            *items[i],
-            &entries_[entries_index],
-            &parent_resource_ids_[entries_index])) {
+    if (items[i]->type() == google_apis::ChangeResource::TEAM_DRIVE) {

[hashimoto, 2017/04/07 07:56:41]

Can't we do this check in ConvertTeamDriveChangeResourceToResourceEntry?
Can't we merge ConvertChangeResourceToResourceEntry to
ConvertTeamDriveChangeResourceToResourceEntry?

This way, we can get rid of team_drives_ by storing all entires in entries_.

[yamaguchi, 2017/04/07 15:31:46]

Done.

+      if (ConvertTeamDriveChangeResourceToResourceEntry(
+              *items[i], &team_drives_[team_drives_index])) {
+        ++team_drives_index;
+      }
+    } else if (ConvertChangeResourceToResourceEntry(
+                   *items[i], &entries_[entries_index],
+                   &parent_resource_ids_[entries_index])) {
       ++entries_index;
     }
   }
   entries_.resize(entries_index);
+  team_drives_.resize(team_drives_index);
   parent_resource_ids_.resize(entries_index);
 }
 
 ChangeList::ChangeList(const google_apis::FileList& file_list)
     : next_url_(file_list.next_link()),
       largest_changestamp_(0) {
   const std::vector<std::unique_ptr<google_apis::FileResource>>& items =
       file_list.items();
   entries_.resize(items.size());
   parent_resource_ids_.resize(items.size());
@@ skipping 41 matching lines @@
 
     DVLOG(1) << "Root folder ID is " << about_resource->root_folder_id();
     DCHECK(!about_resource->root_folder_id().empty());
   }
 
   // Convert ChangeList to map.
   ChangeListToEntryMapUMAStats uma_stats;
   for (size_t i = 0; i < change_lists.size(); ++i) {
     ChangeList* change_list = change_lists[i].get();
 
+    const std::vector<ResourceEntry>& team_drives = change_list->team_drives();
+    for (size_t i = 0; i < team_drives.size(); ++i) {
+      if (team_drives.at(i).deleted())
+        // TODO(yamaguchi): Handle deleting change of Team Drives. Deletion
+        // entries of Team Drive should be filtered in ApplyEntryMap().
+        continue;
+      ApplyTeamDriveChange(team_drives.at(i));
+    }
+
     std::vector<ResourceEntry>* entries = change_list->mutable_entries();
     for (size_t i = 0; i < entries->size(); ++i) {
       ResourceEntry* entry = &(*entries)[i];
 
       // Count the number of files.
       if (!entry->file_info().is_directory()) {
         uma_stats.IncrementNumFiles(
             entry->file_specific_info().is_hosted_document());
       }
       parent_resource_id_map_[entry->resource_id()] =
@@ skipping 27 matching lines @@
     return error;
   }
 
   // Shouldn't record histograms when processing delta update.
   if (!is_delta_update)
     uma_stats.UpdateFileCountUmaHistograms();
 
   return FILE_ERROR_OK;
 }
 
+FileError ChangeListProcessor::ApplyTeamDriveChange(
+    const ResourceEntry& change) {
+  parent_resource_id_map_[change.resource_id()] =
+      util::kDriveTeamDrivesDirVirtualResourceId;
+  return ApplyEntry(change);
+}
+
 FileError ChangeListProcessor::ApplyEntryMap(
     int64_t changestamp,
     std::unique_ptr<google_apis::AboutResource> about_resource) {
   DCHECK(about_resource);
 
   // Create the entry for "My Drive" directory with the latest changestamp.
   ResourceEntry root;
   FileError error = resource_metadata_->GetResourceEntryByPath(
       util::GetDriveMyDriveRootPath(), &root);
   if (error != FILE_ERROR_OK) {
@@ skipping 92 matching lines @@
         }
       }
       it = it_parent;
     }
 
     // Apply the parent first.
     std::reverse(entries.begin(), entries.end());
     for (size_t i = 0; i < entries.size(); ++i) {
       // Skip root entry in the change list. We don't expect servers to send
       // root entry, but we should better be defensive (see crbug.com/297259).
+      // TODO(yamaguchi): Apply this defensive logic to root directories of
+      // every Team Drive as well.
       ResourceEntryMap::iterator it = entries[i];
       if (it->first != root.resource_id()) {
         FileError error = ApplyEntry(it->second);
         if (error != FILE_ERROR_OK) {
           LOG(ERROR) << "ApplyEntry failed: " << FileErrorToString(error)
                      << ", title = " << it->second.title();
           return error;
         }
       }
       entry_map_.erase(it);
@@ skipping 20 matching lines @@
                  << ", resource_id = " << deleted_resource_ids[i];
       return error;
     }
   }
 
   return FILE_ERROR_OK;
 }
 
 FileError ChangeListProcessor::ApplyEntry(const ResourceEntry& entry) {
   DCHECK(!entry.deleted());
+  DCHECK(!entry.resource_id().empty());
   DCHECK(parent_resource_id_map_.count(entry.resource_id()));
   const std::string& parent_resource_id =
       parent_resource_id_map_[entry.resource_id()];
 
   ResourceEntry new_entry(entry);
   FileError error = SetParentLocalIdOfEntry(resource_metadata_, &new_entry,
                                             parent_resource_id);
   if (error != FILE_ERROR_OK)
     return error;
 
@@ skipping 101 matching lines @@
 
 // static
 FileError ChangeListProcessor::SetParentLocalIdOfEntry(
     ResourceMetadata* resource_metadata,
     ResourceEntry* entry,
     const std::string& parent_resource_id) {
   std::string parent_local_id;
   if (parent_resource_id.empty()) {
     // Entries without parents should go under "other" directory.
     parent_local_id = util::kDriveOtherDirLocalId;
+  } else if (parent_resource_id == util::kDriveTeamDrivesDirVirtualResourceId) {

[hashimoto, 2017/04/07 07:56:41]

Instead of introducing this "virtual resource ID", how about doing nothing when
|entry|'s parent_local_id is already set (or when |entry|'s
parent_local_id==kDriveTeamDrivesDirLocalId)?

[yamaguchi, 2017/04/07 15:31:46]

Changed to insert the virtual resource ID to the "/team_drives" entry in
resource_metadata.cc instead.
I think this would be more simple as TeamDrive entries can also be processed by
existing logic evenly for this kind of localID<->recourceID lookup in other
places as well.

[hashimoto, 2017/04/10 11:01:23]

The idea of "virtual resource ID" sounds awkward to me.
Is there any logic which needs to be changed to handle team drive entries
without introducing this virtual ID other than SetParentLocalIdOfEntry()?

[yamaguchi, 2017/04/11 07:08:07]

Thanks. Now I think it should work without introducing the virtual ID.

+    parent_local_id = util::kDriveTeamDrivesDirLocalId;
   } else {
     FileError error = resource_metadata->GetIdByResourceId(
         parent_resource_id, &parent_local_id);
     if (error != FILE_ERROR_OK)
       return error;
   }
   entry->set_parent_local_id(parent_local_id);
   return FILE_ERROR_OK;
 }
 
 void ChangeListProcessor::UpdateChangedDirs(const ResourceEntry& entry) {
   DCHECK(!entry.resource_id().empty());
 
   std::string local_id;
   base::FilePath file_path;
   if (resource_metadata_->GetIdByResourceId(
           entry.resource_id(), &local_id) == FILE_ERROR_OK)
     resource_metadata_->GetFilePath(local_id, &file_path);
 
   if (!file_path.empty()) {
     FileChange::ChangeType type = entry.deleted()
                                       ? FileChange::CHANGE_TYPE_DELETE
                                       : FileChange::CHANGE_TYPE_ADD_OR_UPDATE;
     changed_files_->Update(file_path, entry, type);
   }
 }
 
 }  // namespace internal
 }  // namespace drive