Reporting: Implement cache.

net/reporting/reporting_cache_unittest.cc

+// Copyright 2017 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 "net/reporting/reporting_cache.h"
+
+#include <string>
+
+#include "base/memory/ptr_util.h"
+#include "base/time/time.h"
+#include "base/values.h"
+#include "net/reporting/reporting_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+#include "url/origin.h"
+
+namespace net {
+namespace {
+
+using Client = ReportingCache::Client;
+using Report = ReportingCache::Report;
+
+const GURL kUrl1("https://origin1/path");

[jkarlin, 2017/03/15 18:54:32]

Globals need to be PODs or arrays/structs of PODs, here and below.

https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables

[Julia Tuttle, 2017/03/16 14:50:38]

But it's a unittest x.x

I suppose static members are just as bad as globals? I guess I could make them
tiny getter functions?

[jkarlin, 2017/03/17 15:07:01]

Yes even tests :p It's important that test code is up to the same standard as
production code. People look to test code for examples.

Make the strings into const char []'s and then make the GURLs const members of
your test class and access them from your test class.

+const url::Origin kOrigin1(GURL("https://origin1/"));
+const url::Origin kOrigin2(GURL("https://origin2/"));
+const GURL kEndpoint1("https://endpoint1/");
+const GURL kEndpoint2("https://endpoint2/");
+const std::string kGroup1("group1");
+const std::string kGroup2("group2");
+const std::string kType("default");
+const base::TimeTicks kNow(base::TimeTicks::Now());
+const base::TimeTicks kExpires1(kNow + base::TimeDelta::FromDays(7));
+const base::TimeTicks kExpires2(kExpires1 + base::TimeDelta::FromDays(7));
+
+class ReportingCacheTest : public ::testing::Test {
+ protected:
+  ReportingCache cache_;
+};
+
+TEST_F(ReportingCacheTest, Reports) {
+  std::vector<const Report*> reports;
+  cache_.GetReports(&reports);
+  EXPECT_TRUE(reports.empty());
+
+  cache_.AddReport(kUrl1, kGroup1, kType,
+                   base::MakeUnique<base::DictionaryValue>(), kNow, 0);
+
+  cache_.GetReports(&reports);
+  ASSERT_EQ(1u, reports.size());
+  const Report* report = reports[0];
+  ASSERT_TRUE(report);
+  EXPECT_EQ(kUrl1, report->url);
+  EXPECT_EQ(kGroup1, report->group);
+  EXPECT_EQ(kType, report->type);
+  // TODO(juliatuttle): Check body?
+  EXPECT_EQ(kNow, report->queued);
+  EXPECT_EQ(0, report->attempts);
+  EXPECT_FALSE(report->pending);
+  EXPECT_FALSE(report->doomed);
+
+  cache_.IncrementReportsAttempts(reports);
+
+  cache_.GetReports(&reports);
+  ASSERT_EQ(1u, reports.size());
+  report = reports[0];
+  ASSERT_TRUE(report);
+  EXPECT_EQ(1, report->attempts);
+
+  cache_.RemoveReports(reports);
+
+  cache_.GetReports(&reports);
+  EXPECT_TRUE(reports.empty());
+}
+
+TEST_F(ReportingCacheTest, PendingReports) {
+  cache_.AddReport(kUrl1, kGroup1, kType,
+                   base::MakeUnique<base::DictionaryValue>(), kNow, 0);
+
+  std::vector<const Report*> reports;
+  cache_.GetReports(&reports);
+  ASSERT_EQ(1u, reports.size());
+  EXPECT_FALSE(reports[0]->pending);
+  EXPECT_FALSE(reports[0]->doomed);
+
+  cache_.SetReportsPending(reports);
+  EXPECT_TRUE(reports[0]->pending);
+  EXPECT_FALSE(reports[0]->doomed);
+
+  cache_.RemoveReports(reports);
+  EXPECT_TRUE(reports[0]->pending);
+  EXPECT_TRUE(reports[0]->doomed);
+
+  // After removing report, future calls to GetReports should not return it.
+  std::vector<const Report*> visible_reports;
+  cache_.GetReports(&visible_reports);
+  EXPECT_TRUE(visible_reports.empty());
+  EXPECT_EQ(1u, cache_.GetFullReportCountForTesting());
+
+  // After clearing pending flag, report should be deleted.
+  cache_.ClearReportsPending(reports);
+  EXPECT_EQ(0u, cache_.GetFullReportCountForTesting());
+}
+
+TEST_F(ReportingCacheTest, Endpoints) {
+  cache_.SetClient(kOrigin1, kEndpoint1, false, kGroup1, kExpires1);

[jkarlin, 2017/03/15 18:54:32]

, false /* sub-domains */, kGroup1, ...

[Julia Tuttle, 2017/03/16 14:50:38]

Obsolete.

+
+  const Client* client = FindClientInCache(&cache_, kOrigin1, kEndpoint1);
+  ASSERT_TRUE(client);
+  EXPECT_EQ(kOrigin1, client->origin);
+  EXPECT_EQ(kEndpoint1, client->endpoint);
+  EXPECT_FALSE(client->subdomains);
+  EXPECT_EQ(kGroup1, client->group);
+  EXPECT_EQ(kExpires1, client->expires);
+
+  cache_.SetClient(kOrigin1, kEndpoint1, true, kGroup2, kExpires2);

[jkarlin, 2017/03/15 18:54:32]

Add a comment that this replaces the origin client with different domains,
group, and expiration time

[jkarlin, 2017/03/15 18:54:33]

ditto with the bool coomment

[Julia Tuttle, 2017/03/16 14:50:38]

Done.

[Julia Tuttle, 2017/03/16 14:50:38]

Obsolete.

+
+  client = FindClientInCache(&cache_, kOrigin1, kEndpoint1);
+  ASSERT_TRUE(client);
+  EXPECT_EQ(kOrigin1, client->origin);
+  EXPECT_EQ(kEndpoint1, client->endpoint);
+  EXPECT_TRUE(client->subdomains);
+  EXPECT_EQ(kGroup2, client->group);
+  EXPECT_EQ(kExpires2, client->expires);
+
+  std::vector<const Client*> clients;

[jkarlin, 2017/03/15 18:54:33]

std::vector<const Client*> clients {client};

[Julia Tuttle, 2017/03/16 14:50:38]

Whoa.

Better yet, all three lines as:

  cache_.RemoveClients(std::vector<const ReportingClient*> { client });

+  clients.push_back(client);
+  cache_.RemoveClients(clients);
+
+  client = FindClientInCache(&cache_, kOrigin1, kEndpoint1);
+  EXPECT_FALSE(client);
+}
+
+TEST_F(ReportingCacheTest, GetClientsForOriginAndGroup) {
+  cache_.SetClient(kOrigin1, kEndpoint1, false, kGroup1, kExpires1);

[jkarlin, 2017/03/15 18:54:32]

In fact, you use this enough, why not make that bool into an enum?

[Julia Tuttle, 2017/03/16 14:50:38]

You are irritatingly correct. Done.

+  cache_.SetClient(kOrigin1, kEndpoint2, false, kGroup2, kExpires1);
+  cache_.SetClient(kOrigin2, kEndpoint1, false, kGroup1, kExpires1);
+
+  std::vector<const Client*> clients;
+  cache_.GetClientsForOriginAndGroup(kOrigin1, kGroup1, &clients);
+  ASSERT_EQ(1u, clients.size());
+  const Client* client = clients[0];
+  ASSERT_TRUE(client);
+  EXPECT_EQ(kOrigin1, client->origin);
+  EXPECT_EQ(kGroup1, client->group);
+}
+
+TEST_F(ReportingCacheTest, RemoveClientForOriginAndEndpoint) {
+  cache_.SetClient(kOrigin1, kEndpoint1, false, kGroup1, kExpires1);
+  cache_.SetClient(kOrigin1, kEndpoint2, false, kGroup2, kExpires1);
+  cache_.SetClient(kOrigin2, kEndpoint1, false, kGroup1, kExpires1);
+
+  cache_.RemoveClientForOriginAndEndpoint(kOrigin1, kEndpoint1);
+
+  std::vector<const Client*> clients;
+  cache_.GetClientsForOriginAndGroup(kOrigin1, kGroup1, &clients);
+  EXPECT_TRUE(clients.empty());
+
+  cache_.GetClientsForOriginAndGroup(kOrigin1, kGroup2, &clients);
+  EXPECT_EQ(1u, clients.size());
+
+  cache_.GetClientsForOriginAndGroup(kOrigin2, kGroup1, &clients);
+  EXPECT_EQ(1u, clients.size());
+}
+
+TEST_F(ReportingCacheTest, RemoveClientsForEndpoint) {
+  cache_.SetClient(kOrigin1, kEndpoint1, false, kGroup1, kExpires1);
+  cache_.SetClient(kOrigin1, kEndpoint2, false, kGroup2, kExpires1);
+  cache_.SetClient(kOrigin2, kEndpoint1, false, kGroup1, kExpires1);
+
+  cache_.RemoveClientsForEndpoint(kEndpoint1);
+
+  std::vector<const Client*> clients;
+  cache_.GetClientsForOriginAndGroup(kOrigin1, kGroup1, &clients);
+  EXPECT_TRUE(clients.empty());
+
+  cache_.GetClientsForOriginAndGroup(kOrigin1, kGroup2, &clients);
+  EXPECT_EQ(1u, clients.size());
+
+  cache_.GetClientsForOriginAndGroup(kOrigin2, kGroup1, &clients);
+  EXPECT_TRUE(clients.empty());
+}
+
+}  // namespace
+}  // namespace net