Issue 5165 - Remove SubscriptionPtr

test/FilterEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 230 matching lines @@
         {
           {
             std::lock_guard<std::mutex> lock(downloadStatusChanged.mutex);
             downloadStatusChanged.url = params[1].GetProperty("url").AsString();
           }
           downloadStatusChanged.cv.notify_one();
         }
       });
     }
 
-    SubscriptionPtr EnsureExampleSubscriptionAndForceUpdate(const std::string& apppendToUrl = std::string())
+    Subscription EnsureExampleSubscriptionAndForceUpdate(const std::string& apppendToUrl = std::string())
     {
       if (!filterEngine)
         filterEngine = FilterEngine::Create(jsEngine, createParams);
       auto subscriptionUrl = subscriptionUrlPrefix + apppendToUrl;
       auto subscription = filterEngine->GetSubscription(subscriptionUrl);
-      EXPECT_EQ(0u, subscription->GetProperty("filters").AsList().size()) << subscriptionUrl;
-      EXPECT_TRUE(subscription->GetProperty("downloadStatus").IsNull()) << subscriptionUrl;
-      subscription->UpdateFilters();
+      EXPECT_EQ(0u, subscription.GetProperty("filters").AsList().size()) << subscriptionUrl;
+      EXPECT_TRUE(subscription.GetProperty("downloadStatus").IsNull()) << subscriptionUrl;
+      subscription.UpdateFilters();
       {
         std::unique_lock<std::mutex> lock(downloadStatusChanged.mutex);
         downloadStatusChanged.cv.wait_for(lock,
           /*don't block tests forever*/std::chrono::seconds(5),
           [this, subscriptionUrl]()->bool
         {
           return subscriptionUrl == downloadStatusChanged.url;
         });
         // Basically it's enough to wait only for downloadStatus although there
         // is still some JS code being executed. Any following attempt to work
@@ skipping 52 matching lines @@
   filter.RemoveFromList();
   ASSERT_EQ(0u, filterEngine->GetListedFilters().size());
   ASSERT_FALSE(filter.IsListed());
   filter.RemoveFromList();
   ASSERT_EQ(0u, filterEngine->GetListedFilters().size());
   ASSERT_FALSE(filter.IsListed());
 }
 
 TEST_F(FilterEngineTest, SubscriptionProperties)
 {
-  AdblockPlus::SubscriptionPtr subscription = filterEngine->GetSubscription("foo");
+  AdblockPlus::Subscription subscription = filterEngine->GetSubscription("foo");
 
-  ASSERT_TRUE(subscription->GetProperty("stringFoo").IsUndefined());
-  ASSERT_TRUE(subscription->GetProperty("intFoo").IsUndefined());
-  ASSERT_TRUE(subscription->GetProperty("boolFoo").IsUndefined());
+  ASSERT_TRUE(subscription.GetProperty("stringFoo").IsUndefined());
+  ASSERT_TRUE(subscription.GetProperty("intFoo").IsUndefined());
+  ASSERT_TRUE(subscription.GetProperty("boolFoo").IsUndefined());
 
-  subscription->SetProperty("stringFoo", "y");
-  subscription->SetProperty("intFoo", 24);
-  subscription->SetProperty("boolFoo", true);
-  ASSERT_EQ("y", subscription->GetProperty("stringFoo").AsString());
-  ASSERT_EQ(24, subscription->GetProperty("intFoo").AsInt());
-  ASSERT_TRUE(subscription->GetProperty("boolFoo").AsBool());
+  subscription.SetProperty("stringFoo", "y");
+  subscription.SetProperty("intFoo", 24);
+  subscription.SetProperty("boolFoo", true);
+  ASSERT_EQ("y", subscription.GetProperty("stringFoo").AsString());
+  ASSERT_EQ(24, subscription.GetProperty("intFoo").AsInt());
+  ASSERT_TRUE(subscription.GetProperty("boolFoo").AsBool());
 }
 
 TEST_F(FilterEngineTest, AddRemoveSubscriptions)
 {
   ASSERT_EQ(0u, filterEngine->GetListedSubscriptions().size());
-  AdblockPlus::SubscriptionPtr subscription = filterEngine->GetSubscription("foo");
+  AdblockPlus::Subscription subscription = filterEngine->GetSubscription("foo");
   ASSERT_EQ(0u, filterEngine->GetListedSubscriptions().size());
-  ASSERT_FALSE(subscription->IsListed());
-  subscription->AddToList();
+  ASSERT_FALSE(subscription.IsListed());
+  subscription.AddToList();
   ASSERT_EQ(1u, filterEngine->GetListedSubscriptions().size());
-  ASSERT_EQ(*subscription, *filterEngine->GetListedSubscriptions()[0]);
-  ASSERT_TRUE(subscription->IsListed());
-  subscription->AddToList();
+  ASSERT_EQ(subscription, filterEngine->GetListedSubscriptions()[0]);
+  ASSERT_TRUE(subscription.IsListed());
+  subscription.AddToList();
   ASSERT_EQ(1u, filterEngine->GetListedSubscriptions().size());
-  ASSERT_EQ(*subscription, *filterEngine->GetListedSubscriptions()[0]);
-  ASSERT_TRUE(subscription->IsListed());
-  subscription->RemoveFromList();
+  ASSERT_EQ(subscription, filterEngine->GetListedSubscriptions()[0]);
+  ASSERT_TRUE(subscription.IsListed());
+  subscription.RemoveFromList();
   ASSERT_EQ(0u, filterEngine->GetListedSubscriptions().size());
-  ASSERT_FALSE(subscription->IsListed());
-  subscription->RemoveFromList();
+  ASSERT_FALSE(subscription.IsListed());
+  subscription.RemoveFromList();
   ASSERT_EQ(0u, filterEngine->GetListedSubscriptions().size());
-  ASSERT_FALSE(subscription->IsListed());
+  ASSERT_FALSE(subscription.IsListed());
 }
 
 TEST_F(FilterEngineTest, SubscriptionUpdates)
 {
-  AdblockPlus::SubscriptionPtr subscription = filterEngine->GetSubscription("foo");
-  ASSERT_FALSE(subscription->IsUpdating());
-  subscription->UpdateFilters();
+  AdblockPlus::Subscription subscription = filterEngine->GetSubscription("foo");
+  ASSERT_FALSE(subscription.IsUpdating());
+  subscription.UpdateFilters();
 }
 
 TEST_F(FilterEngineTest, Matches)
 {
   filterEngine->GetFilter("adbanner.gif").AddToList();
   filterEngine->GetFilter("@@notbanner.gif").AddToList();
   filterEngine->GetFilter("tpbanner.gif$third-party").AddToList();
   filterEngine->GetFilter("fpbanner.gif$~third-party").AddToList();
   filterEngine->GetFilter("combanner.gif$domain=example.com").AddToList();
   filterEngine->GetFilter("orgbanner.gif$domain=~example.com").AddToList();
@@ skipping 336 matching lines @@
       documentUrls1));
 }
 
 TEST_F(FilterEngineWithFreshFolder, LangAndAASubscriptionsAreChosenOnFirstRun)
 {
   AppInfo appInfo;
   appInfo.locale = "zh";
   const std::string langSubscriptionUrl = "https://easylist-downloads.adblockplus.org/easylistchina+easylist.txt";
   auto jsEngine = createJsEngine(appInfo);
   auto filterEngine = AdblockPlus::FilterEngine::Create(jsEngine);
-  const auto subscriptions = filterEngine->GetListedSubscriptions();
+  auto subscriptions = filterEngine->GetListedSubscriptions();
   ASSERT_EQ(2u, subscriptions.size());
-  SubscriptionPtr aaSubscription;
-  SubscriptionPtr langSubscription;
-  if (subscriptions[0]->IsAA())
+  std::unique_ptr<Subscription> aaSubscription;
+  std::unique_ptr<Subscription> langSubscription;
+  if (subscriptions[0].IsAA())
   {
-    aaSubscription = subscriptions[0];
-    langSubscription = subscriptions[1];
+    aaSubscription.reset(new Subscription(std::move(subscriptions[0])));

[sergei, 2017/04/24 14:12:36]

I would not expect that android code is never copying the objects, so moving
here is not necessary, just add a copy-ctr.

[hub, 2017/04/24 15:02:42]

Done.

+    langSubscription.reset(new Subscription(std::move(subscriptions[1])));
   }
-  else if (subscriptions[1]->IsAA())
+  else if (subscriptions[1].IsAA())
   {
-    aaSubscription = subscriptions[1];
-    langSubscription = subscriptions[0];
+    aaSubscription.reset(new Subscription(std::move(subscriptions[1])));
+    langSubscription.reset(new Subscription(std::move(subscriptions[0])));
   }
   ASSERT_NE(nullptr, aaSubscription);
   ASSERT_NE(nullptr, langSubscription);
   EXPECT_EQ(langSubscriptionUrl, langSubscription->GetProperty("url").AsString());
   EXPECT_TRUE(filterEngine->IsAAEnabled());
 }
 
 TEST_F(FilterEngineWithFreshFolder, DisableSubscriptionsAutoSelectOnFirstRun)
 {
   auto jsEngine = createJsEngine();
@@ skipping 151 matching lines @@
         // no subscription (because of preconfigured prefs.json and patterns.ini),
         // though it should be enabled by default in a non-test environment, it's tested in
         // corresponding tests.
         const auto subscriptions = filterEngine->GetListedSubscriptions();
         EXPECT_EQ(0u, subscriptions.size()); // no any subscription including AA
         EXPECT_FALSE(filterEngine->IsAAEnabled());
       }
       if (otherSubscriptionsNumber == 1u)
       {
         auto subscription = filterEngine->GetSubscription(kOtherSubscriptionUrl);
-        ASSERT_TRUE(subscription);
-        subscription->AddToList();
+        subscription.AddToList();
         const auto subscriptions = filterEngine->GetListedSubscriptions();
         ASSERT_EQ(1u, subscriptions.size());
-        EXPECT_FALSE(subscriptions[0]->IsAA());
-        EXPECT_EQ(kOtherSubscriptionUrl, subscriptions[0]->GetProperty("url").AsString());
+        EXPECT_FALSE(subscriptions[0].IsAA());
+        EXPECT_EQ(kOtherSubscriptionUrl, subscriptions[0].GetProperty("url").AsString());
       }
       if (isAASatusPresent(aaStatus))
       {
         filterEngine->SetAAEnabled(true); // add AA by enabling it
         if (aaStatus == AAStatus::disabled_present)
         {
           filterEngine->SetAAEnabled(false);
         }
         testSubscriptionState(aaStatus, otherSubscriptionsNumber);
       }
     }
     bool isAASatusPresent(AAStatus aaStatus)
     {
       return aaStatus != AAStatus::absent;
     }
     void testSubscriptionState(AAStatus aaStatus, int otherSubscriptionsNumber)
     {
       if (aaStatus == AAStatus::enabled)
         EXPECT_TRUE(filterEngine->IsAAEnabled());
       else
         EXPECT_FALSE(filterEngine->IsAAEnabled());
 
-      SubscriptionPtr aaSubscription;
-      SubscriptionPtr otherSubscription;
-      const auto subscriptions = filterEngine->GetListedSubscriptions();
-      for (const auto& subscription : subscriptions)
+      std::unique_ptr<Subscription> aaSubscription;
+      std::unique_ptr<Subscription> otherSubscription;
+      auto subscriptions = filterEngine->GetListedSubscriptions();
+      for (auto& subscription : subscriptions)
       {
-        (subscription->IsAA() ? aaSubscription : otherSubscription) = subscription;
+        (subscription.IsAA() ? aaSubscription : otherSubscription) =
+          std::unique_ptr<Subscription>(new Subscription(std::move(subscription)));
       }
       if (otherSubscriptionsNumber == 1u)
       {
         if (isAASatusPresent(aaStatus))
         {
           EXPECT_EQ(2u, subscriptions.size());
           EXPECT_TRUE(aaSubscription);
           EXPECT_TRUE(otherSubscription);
         }
         else
@@ skipping 26 matching lines @@
     const auto parameter = ::testing::get<0>(GetParam());
     uint8_t otherSubscriptionsNumber = ::testing::get<1>(GetParam());
     init(parameter.initialAAStatus, otherSubscriptionsNumber);
 
     if (parameter.action == Action::enable)
       filterEngine->SetAAEnabled(true);
     else if (parameter.action == Action::disable)
       filterEngine->SetAAEnabled(false);
     else if (parameter.action == Action::remove)
     {
-      SubscriptionPtr aaSubscription;
-      for (const auto& subscription : filterEngine->GetListedSubscriptions())
+      std::unique_ptr<Subscription> aaSubscription;
+      for (auto& subscription : filterEngine->GetListedSubscriptions())
       {
-        if (subscription->IsAA())
+        if (subscription.IsAA())
         {
-          aaSubscription = subscription;
+          aaSubscription.reset(new Subscription(std::move(subscription)));
           break;
         }
       }
       ASSERT_TRUE(aaSubscription);
       aaSubscription->RemoveFromList();
     }
 
     testSubscriptionState(parameter.expectedAAStatus, otherSubscriptionsNumber);
   }
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, AbsentCallbackAllowsUpdating)
 {
   createParams.isConnectionAllowedCallback = FilterEngine::IsConnectionAllowedCallback();
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
-  EXPECT_EQ("synchronize_ok", subscription->GetProperty("downloadStatus").AsString());
-  EXPECT_EQ(1u, subscription->GetProperty("filters").AsList().size());
+  EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
+  EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, AllowingCallbackAllowsUpdating)
 {
   // no stored allowed_connection_type preference
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
-  EXPECT_EQ("synchronize_ok", subscription->GetProperty("downloadStatus").AsString());
-  EXPECT_EQ(1u, subscription->GetProperty("filters").AsList().size());
+  EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
+  EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
   auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
   ASSERT_EQ(1u, capturedConnectionTypes.size());
   EXPECT_FALSE(capturedConnectionTypes[0].first);
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, NotAllowingCallbackDoesNotAllowUpdating)
 {
   isConnectionAllowed = false;
   // no stored allowed_connection_type preference
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
-  EXPECT_EQ("synchronize_connection_error", subscription->GetProperty("downloadStatus").AsString());
-  EXPECT_EQ(0u, subscription->GetProperty("filters").AsList().size());
+  EXPECT_EQ("synchronize_connection_error", subscription.GetProperty("downloadStatus").AsString());
+  EXPECT_EQ(0u, subscription.GetProperty("filters").AsList().size());
   auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
   EXPECT_EQ(1u, capturedConnectionTypes.size());
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, PredefinedAllowedConnectionTypeIsPassedToCallback)
 {
   std::string predefinedAllowedConnectionType = "non-metered";
   createParams.preconfiguredPrefs.insert(std::make_pair("allowed_connection_type",
     jsEngine->NewValue(predefinedAllowedConnectionType)));
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
-  EXPECT_EQ("synchronize_ok", subscription->GetProperty("downloadStatus").AsString());
-  EXPECT_EQ(1u, subscription->GetProperty("filters").AsList().size());
+  EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
+  EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
   auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
   ASSERT_EQ(1u, capturedConnectionTypes.size());
   EXPECT_TRUE(capturedConnectionTypes[0].first);
   EXPECT_EQ(predefinedAllowedConnectionType, capturedConnectionTypes[0].second);
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, ConfiguredConnectionTypeIsPassedToCallback)
 {
   // FilterEngine->RemoveSubscription is not usable here because subscriptions
   // are cached internally by URL. So, different URLs are used in diffirent
   // checks.
   {
     std::string predefinedAllowedConnectionType = "non-metered";
     createParams.preconfiguredPrefs.insert(std::make_pair(
       "allowed_connection_type", jsEngine->NewValue(predefinedAllowedConnectionType)));
     auto subscription = EnsureExampleSubscriptionAndForceUpdate();
-    EXPECT_EQ("synchronize_ok", subscription->GetProperty("downloadStatus").AsString());
-    EXPECT_EQ(1u, subscription->GetProperty("filters").AsList().size());
+    EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
+    EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
     auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
     ASSERT_EQ(1u, capturedConnectionTypes.size());
     EXPECT_TRUE(capturedConnectionTypes[0].first);
     EXPECT_EQ(predefinedAllowedConnectionType, capturedConnectionTypes[0].second);
   }
   capturedConnectionTypes.Clear();
   {
     // set no value
     filterEngine->SetAllowedConnectionType(nullptr);
     auto subscription = EnsureExampleSubscriptionAndForceUpdate("subA");
-    EXPECT_EQ("synchronize_ok", subscription->GetProperty("downloadStatus").AsString());
-    EXPECT_EQ(1u, subscription->GetProperty("filters").AsList().size());
+    EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
+    EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
     auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
     ASSERT_EQ(1u, capturedConnectionTypes.size());
     EXPECT_FALSE(capturedConnectionTypes[0].first);
-    subscription->RemoveFromList();
+    subscription.RemoveFromList();
     this->capturedConnectionTypes.Clear();
   }
   capturedConnectionTypes.Clear();
   {
     // set some value
     std::string testConnection = "test connection";
     filterEngine->SetAllowedConnectionType(&testConnection);
     auto subscription = EnsureExampleSubscriptionAndForceUpdate("subB");
-    EXPECT_EQ("synchronize_ok", subscription->GetProperty("downloadStatus").AsString());
-    EXPECT_EQ(1u, subscription->GetProperty("filters").AsList().size());
+    EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
+    EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
     auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
     ASSERT_EQ(1u, capturedConnectionTypes.size());
     EXPECT_TRUE(capturedConnectionTypes[0].first);
     EXPECT_EQ(testConnection, capturedConnectionTypes[0].second);
   }
 }