Media Galleries API Picasa: Modify the album indexer to de-dupe names.

When multiple folders both contain "Image.jpg" for instance, that goes into the same album, this now gets deduped as:

Image.jpg
Image (1).jpg
Image (2).jpg

This is the same deduping scheme as chromium downloads.

BUG=151701

Review URL: https://chromiumcodereview.appspot.com/23164009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218883 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 124 insertions, 23 deletions

diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc
index b3b5833..616943c 100644
--- a/chrome/utility/chrome_content_utility_client.cc
+++ b/chrome/utility/chrome_content_utility_client.cc
@@ -597,11 +597,7 @@ void ChromeContentUtilityClient::OnIndexPicasaAlbumsContents(
     const picasa::AlbumUIDSet& album_uids,
     const std::vector<picasa::FolderINIContents>& folders_inis) {
   picasa::PicasaAlbumsIndexer indexer(album_uids);
-  for (std::vector<picasa::FolderINIContents>::const_iterator it =
-           folders_inis.begin();
-       it != folders_inis.end(); ++it) {
-    indexer.ParseFolderINI(it->folder_path, it->ini_contents);
-  }
+  indexer.ParseFolderINI(folders_inis);
 
   Send(new ChromeUtilityHostMsg_IndexPicasaAlbumsContents_Finished(
       indexer.albums_images()));
diff --git a/chrome/utility/media_galleries/picasa_albums_indexer.cc b/chrome/utility/media_galleries/picasa_albums_indexer.cc
index 87c7a47..abaeb3d 100644
--- a/chrome/utility/media_galleries/picasa_albums_indexer.cc
+++ b/chrome/utility/media_galleries/picasa_albums_indexer.cc
@@ -4,11 +4,14 @@
 
 #include "chrome/utility/media_galleries/picasa_albums_indexer.h"
 
+#include <algorithm>
+#include <utility>
 #include <vector>
 
 #include "base/ini_parser.h"
 #include "base/logging.h"
 #include "base/strings/string_split.h"
+#include "base/strings/stringprintf.h"
 
 namespace picasa {
 
@@ -16,6 +19,7 @@ namespace {
 
 const char kAlbumSectionHeader[] = ".album:";
 const char kAlbumsKey[] = "albums";
+const int kMaxDedupeNumber = 1000;  // Chosen arbitrarily.
 
 class PicasaINIParser : public base::INIParser {
  public:
@@ -48,8 +52,28 @@ class PicasaINIParser : public base::INIParser {
       if (album_map_it == albums_images_->end())
         continue;
 
-      album_map_it->second.insert(
-          folder_path_.Append(base::FilePath::FromUTF8Unsafe(section)));
+      base::FilePath filename = base::FilePath::FromUTF8Unsafe(section);
+      AlbumImages& album_images = album_map_it->second;
+
+      // If filename is first of its name in album, simply add.
+      if (album_images.insert(
+              std::make_pair(section, folder_path_.Append(filename))).second) {
+        continue;
+      }
+
+      // Otherwise, de-dupe by appending a number starting at (1).
+      for (int i = 1; i < kMaxDedupeNumber; ++i) {
+        std::string deduped_filename =
+            filename.InsertBeforeExtensionASCII(base::StringPrintf(" (%d)", i))
+                .AsUTF8Unsafe();
+
+        // Attempt to add the de-duped name.
+        if (album_images.insert(
+                std::make_pair(deduped_filename, folder_path_.Append(filename)))
+                .second) {
+          break;
+        }
+      }
     }
   }
 
@@ -70,9 +94,20 @@ PicasaAlbumsIndexer::PicasaAlbumsIndexer(const AlbumUIDSet& album_uids) {
 PicasaAlbumsIndexer::~PicasaAlbumsIndexer() {}
 
 void PicasaAlbumsIndexer::ParseFolderINI(
-    const base::FilePath& folder_path, const std::string& ini_contents) {
-  PicasaINIParser parser(folder_path, &albums_images_);
-  parser.Parse(ini_contents);
+    const std::vector<picasa::FolderINIContents>& folders_inis) {
+  // Make a copy for sorting
+  std::vector<picasa::FolderINIContents> folders_inis_sorted = folders_inis;
+
+  // Sort here so image names are deduplicated in a stable ordering.
+  std::sort(folders_inis_sorted.begin(), folders_inis_sorted.end());
+
+  for (std::vector<picasa::FolderINIContents>::const_iterator it =
+           folders_inis_sorted.begin();
+       it != folders_inis_sorted.end();
+       ++it) {
+    PicasaINIParser parser(it->folder_path, &albums_images_);
+    parser.Parse(it->ini_contents);
+  }
 }
 
 }  // namespace picasa
diff --git a/chrome/utility/media_galleries/picasa_albums_indexer.h b/chrome/utility/media_galleries/picasa_albums_indexer.h
index 9cb180f..0eead42 100644
--- a/chrome/utility/media_galleries/picasa_albums_indexer.h
+++ b/chrome/utility/media_galleries/picasa_albums_indexer.h
@@ -48,8 +48,8 @@ class PicasaAlbumsIndexer {
   ~PicasaAlbumsIndexer();
 
   // This method should be called once for each Folder in the PMP database.
-  void ParseFolderINI(const base::FilePath& folder_path,
-                      const std::string& ini_contents);
+  void ParseFolderINI(
+      const std::vector<picasa::FolderINIContents>& folders_inis);
 
   const AlbumImagesMap& albums_images() const { return albums_images_; }
 
diff --git a/chrome/utility/media_galleries/picasa_albums_indexer_unittest.cc b/chrome/utility/media_galleries/picasa_albums_indexer_unittest.cc
index 5714d0c..83ab66f 100644
--- a/chrome/utility/media_galleries/picasa_albums_indexer_unittest.cc
+++ b/chrome/utility/media_galleries/picasa_albums_indexer_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/files/file_path.h"
+#include "chrome/common/media_galleries/picasa_types.h"
 #include "chrome/utility/media_galleries/picasa_albums_indexer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -18,7 +19,7 @@ const char test_ini_string[] =
     "[InBoth.jpg]\n"
     "albums=e66fb059001aabcc69b262b7009fad90,18cb2df48aaa98e1c276b45cfcd81c95\n"
     "[.album:18cb2df48aaa98e1c276b45cfcd81c95]\n"
-    "name=CustomAlbum1\n"
+    "name=CustomAlbum2\n"
     "token=18cb2df48aaa98e1c276b45cfcd81c95\n"
     "date=2013-04-01T16:37:34-07:00\n"
     "[InFirst.jpg]\n"
@@ -32,8 +33,12 @@ TEST(PicasaAlbumsIndexerTest, ValidCase) {
   valid_album_uuids.insert("18cb2df48aaa98e1c276b45cfcd81c95");
   PicasaAlbumsIndexer indexer(valid_album_uuids);
 
-  base::FilePath folder_dir(FILE_PATH_LITERAL("/foo/baz"));
-  indexer.ParseFolderINI(folder_dir, test_ini_string);
+  std::vector<FolderINIContents> folders_inis;
+  FolderINIContents folder_ini;
+  folder_ini.folder_path = base::FilePath(FILE_PATH_LITERAL("/foo/baz"));
+  folder_ini.ini_contents = test_ini_string;
+  folders_inis.push_back(folder_ini);
+  indexer.ParseFolderINI(folders_inis);
 
   const AlbumImagesMap& albums_images = indexer.albums_images();
   AlbumImagesMap::const_iterator it;
@@ -41,14 +46,75 @@ TEST(PicasaAlbumsIndexerTest, ValidCase) {
   it = albums_images.find("e66fb059001aabcc69b262b7009fad90");
   ASSERT_NE(albums_images.end(), it);
   EXPECT_EQ(2u, it->second.size());
-  EXPECT_EQ(1u, it->second.count(folder_dir.AppendASCII("InBoth.jpg")));
-  EXPECT_EQ(1u, it->second.count(folder_dir.AppendASCII("InFirst.jpg")));
+  EXPECT_EQ(1u, it->second.count("InBoth.jpg"));
+  EXPECT_EQ(1u, it->second.count("InFirst.jpg"));
 
   it = albums_images.find("18cb2df48aaa98e1c276b45cfcd81c95");
   ASSERT_NE(albums_images.end(), it);
   EXPECT_EQ(2u, it->second.size());
-  EXPECT_EQ(1u, it->second.count(folder_dir.AppendASCII("InBoth.jpg")));
-  EXPECT_EQ(1u, it->second.count(folder_dir.AppendASCII("InSecond.jpg")));
+  EXPECT_EQ(1u, it->second.count("InBoth.jpg"));
+  EXPECT_EQ(1u, it->second.count("InSecond.jpg"));
+}
+
+TEST(PicasaAlbumsIndexerTest, DedupeNames) {
+  std::set<std::string> valid_album_uuids;
+  valid_album_uuids.insert("e66fb059001aabcc69b262b7009fad90");
+  valid_album_uuids.insert("18cb2df48aaa98e1c276b45cfcd81c95");
+  PicasaAlbumsIndexer indexer(valid_album_uuids);
+
+  std::vector<FolderINIContents> folders_inis;
+  FolderINIContents folder_ini;
+  folder_ini.folder_path = base::FilePath(FILE_PATH_LITERAL("/foo/baz"));
+  folder_ini.ini_contents = test_ini_string;
+  folders_inis.push_back(folder_ini);
+
+  // Add a second folder with just a duplicate InFirst.jpg.
+  folder_ini.folder_path = base::FilePath(FILE_PATH_LITERAL("/foo/second"));
+  folder_ini.ini_contents =
+      "[.album:e66fb059001aabcc69b262b7009fad90]\n"
+      "name=CustomAlbum1\n"
+      "token=e66fb059001aabcc69b262b7009fad90\n"
+      "date=2013-03-15T14:53:21-07:00\n"
+      "[InFirst.jpg]\n"
+      "albums=e66fb059001aabcc69b262b7009fad90\n";
+  folders_inis.push_back(folder_ini);
+
+  // Add a third folder with just a duplicate InFirst.jpg and InSecond.jpg.
+  folder_ini.folder_path = base::FilePath(FILE_PATH_LITERAL("/foo/third"));
+  folder_ini.ini_contents =
+      "[.album:e66fb059001aabcc69b262b7009fad90]\n"
+      "name=CustomAlbum1\n"
+      "token=e66fb059001aabcc69b262b7009fad90\n"
+      "date=2013-03-15T14:53:21-07:00\n"
+      "[.album:18cb2df48aaa98e1c276b45cfcd81c95]\n"
+      "name=CustomAlbum2\n"
+      "token=18cb2df48aaa98e1c276b45cfcd81c95\n"
+      "date=2013-04-01T16:37:34-07:00\n"
+      "[InFirst.jpg]\n"
+      "albums=e66fb059001aabcc69b262b7009fad90\n"
+      "[InSecond.jpg]\n"
+      "albums=18cb2df48aaa98e1c276b45cfcd81c95\n";
+  folders_inis.push_back(folder_ini);
+
+  indexer.ParseFolderINI(folders_inis);
+
+  const AlbumImagesMap& albums_images = indexer.albums_images();
+  AlbumImagesMap::const_iterator it;
+
+  it = albums_images.find("e66fb059001aabcc69b262b7009fad90");
+  ASSERT_NE(albums_images.end(), it);
+  EXPECT_EQ(4u, it->second.size());
+  EXPECT_EQ(1u, it->second.count("InBoth.jpg"));
+  EXPECT_EQ(1u, it->second.count("InFirst.jpg"));
+  EXPECT_EQ(1u, it->second.count("InFirst (1).jpg"));
+  EXPECT_EQ(1u, it->second.count("InFirst (2).jpg"));
+
+  it = albums_images.find("18cb2df48aaa98e1c276b45cfcd81c95");
+  ASSERT_NE(albums_images.end(), it);
+  EXPECT_EQ(3u, it->second.size());
+  EXPECT_EQ(1u, it->second.count("InBoth.jpg"));
+  EXPECT_EQ(1u, it->second.count("InSecond.jpg"));
+  EXPECT_EQ(1u, it->second.count("InSecond (1).jpg"));
 }
 
 TEST(PicasaAlbumsIndexerTest, EdgeCases) {
@@ -57,8 +123,12 @@ TEST(PicasaAlbumsIndexerTest, EdgeCases) {
   valid_album_uuids.insert("18cb2df48aaa98e1c276b45cfcd81c95");
   PicasaAlbumsIndexer indexer(valid_album_uuids);
 
-  base::FilePath folder_dir(FILE_PATH_LITERAL("/foo/baz"));
-  indexer.ParseFolderINI(folder_dir, test_ini_string);
+  std::vector<FolderINIContents> folders_inis;
+  FolderINIContents folder_ini;
+  folder_ini.folder_path = base::FilePath(FILE_PATH_LITERAL("/foo/baz"));
+  folder_ini.ini_contents = test_ini_string;
+  folders_inis.push_back(folder_ini);
+  indexer.ParseFolderINI(folders_inis);
 
   const AlbumImagesMap& albums_images = indexer.albums_images();
   AlbumImagesMap::const_iterator it;
@@ -76,8 +146,8 @@ TEST(PicasaAlbumsIndexerTest, EdgeCases) {
   it = albums_images.find("18cb2df48aaa98e1c276b45cfcd81c95");
   EXPECT_NE(albums_images.end(), it);
   EXPECT_EQ(2u, it->second.size());
-  EXPECT_EQ(1u, it->second.count(folder_dir.AppendASCII("InBoth.jpg")));
-  EXPECT_EQ(1u, it->second.count(folder_dir.AppendASCII("InSecond.jpg")));
+  EXPECT_EQ(1u, it->second.count("InBoth.jpg"));
+  EXPECT_EQ(1u, it->second.count("InSecond.jpg"));
 }
 
 }  // namespace