Modified IIconResolutionService: added new method HaveResolvedBefore() and HaveFailedBefore(). #164

Author: end2endzone

src/core/IIconResolutionService.h

@@ -56,6 +56,20 @@ namespace shellanything
     /// <returns>Returns true if the operation is successful. Returns false otherwise.</returns>
     virtual bool ResolveFileExtensionIcon(Icon & icon) = 0;
 
+    /// <summary>
+    /// Check if the given file extension have resolved before.
+    /// </summary>
+    /// <param name="file_entension">The file extension to check.</param>
+    /// <returns>Returns true if the file extension has previously resolve to a valid icon. Returns false otherwise.</returns>
+    virtual bool HaveResolvedBefore(const std::string& file_extension) const = 0;
+
+    /// <summary>
+    /// Check if the given file extension have previously failed to resolve.
+    /// </summary>
+    /// <param name="file_entension">The file extension to check.</param>
+    /// <returns>Returns true if the file extension has previously failed to resolve. Returns false otherwise.</returns>
+    virtual bool HaveFailedBefore(const std::string& file_extension) const = 0;
+
   };
 
 } //namespace shellanything

src/core/Icon.cpp

@@ -86,6 +86,31 @@ namespace shellanything
 
   void Icon::ResolveFileExtensionIcon()
   {
+    IIconResolutionService* icon_resolution_service = App::GetInstance().GetIconResolutionService();
+    if (icon_resolution_service == NULL)
+    {
+      SA_LOG(ERROR) << "No Icon Resolution service configured for resolving file extensions to icon.";
+      return;
+    }
+
+    shellanything::PropertyManager& pmgr = shellanything::PropertyManager::GetInstance();
+    std::string file_extension = pmgr.Expand(mFileExtension);
+
+    const bool have_resolved_before = icon_resolution_service->HaveResolvedBefore(file_extension);
+    const bool have_failed_before = icon_resolution_service->HaveFailedBefore(file_extension);
+
+    //Do the actual resolution
+    bool success = icon_resolution_service->ResolveFileExtensionIcon(*this);
+
+    // Print a success/failure message once. Issue #98.
+    if (success && !have_resolved_before)
+    {
+      SA_LOG(INFO) << "Resolving icon for file extension '" << file_extension << "' to file '" << mPath << "' with index '" << mIndex << "'";
+    }
+    else if (!success && !have_failed_before)
+    {
+      SA_LOG(WARNING) << "Failed to resolve icon for file extension '" << file_extension << "'.";
+    }
   }
 
   const std::string& Icon::GetFileExtension() const

src/tests/TestIcon.cpp

@@ -240,7 +240,7 @@ namespace shellanything
       ASSERT_EQ(UNKNOWN_ICON_INDEX, icon.GetIndex());
 
       //act (issue #98)
-      for (int i = 0; i < 500; i++)
+      for (int i = 0; i < 100; i++)
       {
         //this should create multiple log entries if the feature is not properly implemented.
         Icon tmp_icon;
@@ -263,7 +263,8 @@ namespace shellanything
           ASSERT_TRUE(read) << "Failed to read log file: " << path;
 
           int count = CountString(content, UNKNOWN_FILE_EXTENSION);
-          ASSERT_LE(count, 1) << "Log file '" << path << "' contains " << count << " references to '" << UNKNOWN_FILE_EXTENSION << "'.";
+          static const int MAX_COUNT = 2; // Issue 167. A maximum of 2 reference is allowed. One from LegacyIconResolutionService class and another from Icon::ResolveFileExtensionIcon().
+          ASSERT_LE(count, MAX_COUNT) << "Log file '" << path << "' contains " << count << " references to '" << UNKNOWN_FILE_EXTENSION << "'.";
         }
       }
     }

src/windows/LegacyIconResolutionService.cpp

@@ -34,8 +34,6 @@
 
 namespace shellanything
 {
-  LegacyIconResolutionService::FileExtensionSet LegacyIconResolutionService::mUnresolvedFileExtensions;
-
   LegacyIconResolutionService::LegacyIconResolutionService()
   {
   }
@@ -81,6 +79,8 @@ namespace shellanything
         icon.SetIndex(resolved_icon.index);
         icon.SetFileExtension("");
 
+        mResolvedFileExtensions.insert(file_extension);
+
         return true;
       }
       else
@@ -92,8 +92,7 @@ namespace shellanything
         icon.SetIndex(unknown_file_icon.index);
         icon.SetFileExtension("");
 
-        const bool is_already_in_log = mUnresolvedFileExtensions.find(file_extension) != mUnresolvedFileExtensions.end();
-        if (!is_already_in_log)
+        if (!HaveFailedBefore(file_extension))
         {
           SA_LOG(WARNING) << "Failed to find icon for file extension '" << file_extension << "'. Resolving icon with default icon for unknown file type '" << unknown_file_icon.path << "' with index '" << unknown_file_icon.index << "'";
 
@@ -108,4 +107,21 @@ namespace shellanything
     return false;
   }
 
+  bool LegacyIconResolutionService::HaveResolvedBefore(const std::string& file_extension) const
+  {
+    const bool has_resolved_before = mResolvedFileExtensions.find(file_extension) != mResolvedFileExtensions.end();
+    if (has_resolved_before)
+      return true;
+
+    // or did we resolved as a failure ?
+    const bool has_already_failed = HaveFailedBefore(file_extension);
+    return has_already_failed;
+  }
+
+  bool LegacyIconResolutionService::HaveFailedBefore(const std::string& file_extension) const
+  {
+    const bool has_already_failed = mUnresolvedFileExtensions.find(file_extension) != mUnresolvedFileExtensions.end();
+    return has_already_failed;
+  }
+
 } //namespace shellanything

src/windows/LegacyIconResolutionService.h

@@ -52,9 +52,24 @@ namespace shellanything
     /// <returns>Returns true if the operation is successful. Returns false otherwise.</returns>
     virtual bool ResolveFileExtensionIcon(Icon& icon);
 
+    /// <summary>
+    /// Check if the given file extension have resolved before.
+    /// </summary>
+    /// <param name="file_entension">The file extension to check.</param>
+    /// <returns>Returns true if the file extension has previously resolve to a valid icon. Returns false otherwise.</returns>
+    virtual bool HaveResolvedBefore(const std::string& file_extension) const;
+
+    /// <summary>
+    /// Check if the given file extension have previously failed to resolve.
+    /// </summary>
+    /// <param name="file_extension">The file extension to check.</param>
+    /// <returns>Returns true if the file extension has previously failed to resolve. Returns false otherwise.</returns>
+    virtual bool HaveFailedBefore(const std::string& file_extension) const;
+
   private:
     typedef std::set<std::string /*file extension*/> FileExtensionSet;
-    static FileExtensionSet mUnresolvedFileExtensions;
+    FileExtensionSet mResolvedFileExtensions;
+    FileExtensionSet mUnresolvedFileExtensions;
 
   };