Add guards to metric values; erase bad events/metrics from db

chrome/browser/performance_monitor/database_unittest.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 <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/time.h"
 #include "chrome/browser/performance_monitor/database.h"
+#include "chrome/browser/performance_monitor/key_builder.h"
 #include "chrome/browser/performance_monitor/metric.h"
 #include "chrome/browser/performance_monitor/performance_monitor_util.h"
 #include "chrome/common/extensions/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
+#include "third_party/leveldatabase/src/include/leveldb/iterator.h"
 
 using extensions::Extension;
 
 namespace performance_monitor {
 
+// A class which is friended by Database, in order to hold the private methods
+// and avoid friending all the different test classes.
+class DatabaseTestHelper {
+ public:
+  explicit DatabaseTestHelper(Database* database) : database_(database) { };
+  ~DatabaseTestHelper() { };
+
+  bool Close() { return database_->Close(); }
+
+  // Override the check for a metric's validity and insert it in the database.
+  // Note: This does not do extraneous updates, like updating the recent_db or
+  // the max_value_map.
+  bool AddInvalidMetric(std::string activity, Metric metric) {
+    std::string metric_key =
+        database_->key_builder_->CreateMetricKey(database_->clock_->GetTime(),
+                                                 metric.type,
+                                                 activity);
+    leveldb::Status status =
+        database_->metric_db_->Put(database_->write_options_,
+                                   metric_key,
+                                   metric.ValueAsString());
+    return status.ok();
+  }
+
+  // Writes an invalid event to the database; since events are stored as JSON
+  // strings, this is equivalent to writing a garbage string.
+  bool AddInvalidEvent(base::Time time, EventType type) {
+    std::string key = database_->key_builder_->CreateEventKey(time, type);
+    leveldb::Status status =
+        database_->event_db_->Put(database_->write_options_, key, "fake_event");
+    return status.ok();
+  }
+
+  size_t GetNumberOfMetricEntries() {
+    return GetNumberOfEntries(database_->metric_db_.get());
+  }
+  size_t GetNumberOfEventEntries() {
+    return GetNumberOfEntries(database_->event_db_.get());
+  }
+
+ private:
+  // Returns the number of entries in a given database.
+  size_t GetNumberOfEntries(leveldb::DB* level_db) {
+    size_t number_of_entries = 0;
+    scoped_ptr<leveldb::Iterator> iter(
+        level_db->NewIterator(database_->read_options_));
+    for (iter->SeekToFirst(); iter->Valid(); iter->Next())
+      ++number_of_entries;
+    return number_of_entries;
+  }
+
+  Database* database_;
+};
+
 // A clock that increments every access. Great for testing.
 class TestingClock : public Database::Clock {
  public:
   TestingClock()
       : counter_(0) {
   }
   explicit TestingClock(const TestingClock& other)
       : counter_(other.counter_) {
   }
   virtual ~TestingClock() {}
@@ skipping 66 matching lines @@
     db_->set_clock(scoped_ptr<Database::Clock>(clock_));
     activity_ = std::string("A");
   }
 
   void SetUp() {
     ASSERT_TRUE(db_.get());
     PopulateDB();
   }
 
   void PopulateDB() {
-    db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE,
-                   std::string("50.5"));
-    db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("13.1"));
-
-    db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
-                   std::string("1000000"));
-    db_->AddMetric(activity_, METRIC_PRIVATE_MEMORY_USAGE,
-                   std::string("3000000"));
+    db_->AddMetric(kProcessChromeAggregate,
+                   Metric(METRIC_CPU_USAGE, clock_->GetTime(), 50.5));
+    db_->AddMetric(activity_,
+                   Metric(METRIC_CPU_USAGE, clock_->GetTime(), 13.1));
+    db_->AddMetric(kProcessChromeAggregate,
+                   Metric(METRIC_PRIVATE_MEMORY_USAGE,
+                          clock_->GetTime(),
+                          1000000.0));
+    db_->AddMetric(activity_,
+                   Metric(METRIC_PRIVATE_MEMORY_USAGE,
+                          clock_->GetTime(),
+                          3000000.0));
   }
 
   scoped_ptr<Database> db_;
   Database::Clock* clock_;
   ScopedTempDir temp_dir_;
   std::string activity_;
 };
 
 ////// PerformanceMonitorDatabaseSetupTests ////////////////////////////////////
 TEST(PerformanceMonitorDatabaseSetupTest, OpenClose) {
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   scoped_ptr<Database> db = Database::Create(temp_dir.path());
   ASSERT_TRUE(db.get());
-  ASSERT_TRUE(db->Close());
+  DatabaseTestHelper helper(db.get());
+  ASSERT_TRUE(helper.Close());
 }
 
 TEST(PerformanceMonitorDatabaseSetupTest, ActiveInterval) {
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   TestingClock* clock_1 = new TestingClock();
   base::Time start_time = clock_1->GetTime();
 
   scoped_ptr<Database> db_1 = Database::Create(temp_dir.path());
   db_1->set_clock(scoped_ptr<Database::Clock>(clock_1));
+  DatabaseTestHelper helper1(db_1.get());
   db_1->AddStateValue("test", "test");
   ASSERT_TRUE(db_1.get());
-  ASSERT_TRUE(db_1->Close());
+  ASSERT_TRUE(helper1.Close());
 
   TestingClock* clock_2 = new TestingClock(*clock_1);
   base::Time mid_time = clock_2->GetTime();
   scoped_ptr<Database> db_2 = Database::Create(temp_dir.path());
   db_2->set_clock(scoped_ptr<Database::Clock>(clock_2));
+  DatabaseTestHelper helper2(db_2.get());
   db_2->AddStateValue("test", "test");
   ASSERT_TRUE(db_2.get());
-  ASSERT_TRUE(db_2->Close());
+  ASSERT_TRUE(helper2.Close());
 
   TestingClock* clock_3 = new TestingClock(*clock_2);
   base::Time end_time = clock_3->GetTime();
   scoped_ptr<Database> db_3 = Database::Create(temp_dir.path());
   db_3->set_clock(scoped_ptr<Database::Clock>(clock_3));
   db_3->AddStateValue("test", "test");
   ASSERT_TRUE(db_3.get());
 
   std::vector<TimeRange> active_interval = db_3->GetActiveIntervals(start_time,
                                                                     end_time);
@@ skipping 53 matching lines @@
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetMaxMetric) {
   Metric stat;
   EXPECT_EQ(0.0, db_->GetMaxStatsForActivityAndMetric(activity_,
                                                       METRIC_PAGE_LOAD_TIME));
 
   EXPECT_EQ(1000000,
             db_->GetMaxStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE));
 
-  db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
-                 std::string("99"));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_PRIVATE_MEMORY_USAGE, clock_->GetTime(), 99.0));
   EXPECT_EQ(1000000,
             db_->GetMaxStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE));
 
-  db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
-                 std::string("6000000"));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_PRIVATE_MEMORY_USAGE,
+                        clock_->GetTime(),
+                        6000000.0));
   EXPECT_EQ(6000000,
             db_->GetMaxStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE));
 }
 
 TEST_F(PerformanceMonitorDatabaseEventTest, GetAllEventTypes) {
   Database::EventTypeSet types = db_->GetEventTypes();
   ASSERT_EQ(2u, types.size());
   ASSERT_EQ(1u, types.count(EVENT_EXTENSION_INSTALL));
   ASSERT_EQ(1u, types.count(EVENT_EXTENSION_UNINSTALL));
 }
 
 TEST_F(PerformanceMonitorDatabaseEventTest, GetEventInTimeRange) {
   base::Time start_time = clock_->GetTime();
   scoped_ptr<Event> crash_event = util::CreateRendererFreezeEvent(
       clock_->GetTime(), "chrome://freeze");
   db_->AddEvent(*crash_event.get());
   Database::EventVector events = db_->GetEvents(start_time, clock_->GetTime());
   ASSERT_EQ(1u, events.size());
   EXPECT_TRUE(events[0]->data()->Equals(crash_event->data()));
 }
 
 TEST_F(PerformanceMonitorDatabaseEventTest, GetInstallEvents) {
   Database::EventVector events = db_->GetEvents(EVENT_EXTENSION_INSTALL);
   ASSERT_EQ(2u, events.size());
   EXPECT_TRUE(events[0]->data()->Equals(install_event_1_->data()));
   EXPECT_TRUE(events[1]->data()->Equals(install_event_2_->data()));
 }
 
+TEST_F(PerformanceMonitorDatabaseEventTest, InvalidEvents) {
+  DatabaseTestHelper helper(db_.get());
+
+  // Insert an invalid event into the database; verify it is inserted.
+  size_t original_number_of_entries = helper.GetNumberOfEventEntries();
+  ASSERT_TRUE(helper.AddInvalidEvent(
+      clock_->GetTime(), EVENT_EXTENSION_INSTALL));
+  ASSERT_EQ(original_number_of_entries + 1u, helper.GetNumberOfEventEntries());
+
+  // Should not retrieve the invalid event.
+  Database::EventVector events = db_->GetEvents();
+  ASSERT_EQ(original_number_of_entries, events.size());
+
+  // Invalid event should have been deleted.
+  ASSERT_EQ(original_number_of_entries, helper.GetNumberOfEventEntries());
+}
+
 TEST_F(PerformanceMonitorDatabaseEventTest, GetUnusedEventType) {
   Database::EventVector events = db_->GetEvents(EVENT_EXTENSION_DISABLE);
   ASSERT_TRUE(events.empty());
   events = db_->GetEvents(EVENT_EXTENSION_DISABLE, clock_->GetTime(),
                           clock_->GetTime());
   ASSERT_TRUE(events.empty());
 }
 
 TEST_F(PerformanceMonitorDatabaseEventTest, GetEventsTimeRange) {
   base::Time start_time = clock_->GetTime();
@@ skipping 63 matching lines @@
   db_->AddStateValue(key, value_2);
   EXPECT_EQ(db_->GetStateValue(key), value_2);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForActivityAndMetric) {
   Database::MetricVector stats = db_->GetStatsForActivityAndMetric(
       activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("18"));
+  db_->AddMetric(activity_,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 18.0));
   stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_CPU_USAGE,
                                             before, clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(18, stats[0].value);
   stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_CPU_USAGE);
   ASSERT_EQ(2u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   EXPECT_EQ(18, stats[1].value);
   stats = db_->GetStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE);
   ASSERT_EQ(1u, stats.size());
@@ skipping 11 matching lines @@
   linked_ptr<Database::MetricVector> stats = stats_map[activity_];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(13.1, stats->at(0).value);
   stats = stats_map[kProcessChromeAggregate];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(50.5, stats->at(0).value);
   stats_map = db_->GetStatsForMetricByActivity(
       METRIC_CPU_USAGE, clock_->GetTime(), clock_->GetTime());
   EXPECT_EQ(0u, stats_map.size());
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("18"));
+  db_->AddMetric(activity_,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 18.0));
   stats_map = db_->GetStatsForMetricByActivity(METRIC_CPU_USAGE, before,
                                                clock_->GetTime());
   ASSERT_EQ(1u, stats_map.size());
   stats = stats_map[activity_];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(18, stats->at(0).value);
 }
 
+TEST_F(PerformanceMonitorDatabaseMetricTest, InvalidMetrics) {
+  DatabaseTestHelper helper(db_.get());
+  Metric invalid_metric(METRIC_CPU_USAGE, clock_->GetTime(), -5.0);
+  ASSERT_FALSE(invalid_metric.Validate());
+
+  // Check that the database normally refuses to insert bad metrics.
+  EXPECT_FALSE(db_->AddMetric(invalid_metric));
+
+  // Verify that it was not inserted into the database.
+  Database::MetricVector stats = db_->GetStatsForActivityAndMetric(
+      activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
+  ASSERT_EQ(1u, stats.size());

[Yoyo Zhou, 2012/09/10 23:52:44]

Should this 1u be the original_number_of_entries?

[Devlin, 2012/09/11 18:52:17]

Done.

+
+  // Forcefully insert it into the database.
+  size_t original_number_of_entries = helper.GetNumberOfMetricEntries();
+  ASSERT_TRUE(helper.AddInvalidMetric(activity_, invalid_metric));
+  ASSERT_EQ(original_number_of_entries + 1u, helper.GetNumberOfMetricEntries());
+
+  // Try to retrieve it; should only get one result.
+  stats = db_->GetStatsForActivityAndMetric(
+      activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
+  ASSERT_EQ(1u, stats.size());
+
+  // Entry should have been deleted in the database.
+  ASSERT_EQ(original_number_of_entries, helper.GetNumberOfMetricEntries());
+}
+
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetFullRange) {
-  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("3.4"));
-  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("21"));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 3.4));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 21.0));
   Database::MetricVector stats =
       db_->GetStatsForActivityAndMetric(METRIC_CPU_USAGE);
   ASSERT_EQ(3u, stats.size());
   ASSERT_EQ(50.5, stats[0].value);
   ASSERT_EQ(3.4, stats[1].value);
   ASSERT_EQ(21, stats[2].value);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetRange) {
   base::Time start = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("3"));
-  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("9"));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 3.0));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 9.0));
   base::Time end = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("21"));
+  db_->AddMetric(kProcessChromeAggregate,
+                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 21.0));
   Database::MetricVector stats =
       db_->GetStatsForActivityAndMetric(METRIC_CPU_USAGE, start, end);
   ASSERT_EQ(2u, stats.size());
   ASSERT_EQ(3, stats[0].value);
   ASSERT_EQ(9, stats[1].value);
 }
 
 }  // namespace performance_monitor