Commit 0d89c8a2 authored by Tommy Nyquist's avatar Tommy Nyquist Committed by Commit Bot

Add new validator for whether In-Product Help events should be stored.

This CL adds a new interface that will be used to validate whether new
events should be stored, and during DB load on startup, it is also used
to check if old event should still be kept around.

The production implementation of this will be
FeatureConfigStorageValidator, which uses the FeatureConfig to check
what should be stored, but for now, the NeverStorageValidator will be
used for all things to ensure that nothing is ever stored for now.

This CL sets up the ownership of the StorageValidator, but does not
update the ModelImpl to use the new framework, and leaves that as
TODOs.

BUG=706309

Change-Id: Ic581edc73f8f059c18fb1dcb5a549c4e62f490f8
Reviewed-on: https://chromium-review.googlesource.com/484682Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#466875}
parent 2ec8db55
...@@ -21,6 +21,8 @@ static_library("internal") { ...@@ -21,6 +21,8 @@ static_library("internal") {
"configuration.h", "configuration.h",
"editable_configuration.cc", "editable_configuration.cc",
"editable_configuration.h", "editable_configuration.h",
"feature_config_storage_validator.cc",
"feature_config_storage_validator.h",
"feature_constants.cc", "feature_constants.cc",
"feature_engagement_tracker_impl.cc", "feature_engagement_tracker_impl.cc",
"feature_engagement_tracker_impl.h", "feature_engagement_tracker_impl.h",
...@@ -33,10 +35,13 @@ static_library("internal") { ...@@ -33,10 +35,13 @@ static_library("internal") {
"model_impl.h", "model_impl.h",
"never_condition_validator.cc", "never_condition_validator.cc",
"never_condition_validator.h", "never_condition_validator.h",
"never_storage_validator.cc",
"never_storage_validator.h",
"once_condition_validator.cc", "once_condition_validator.cc",
"once_condition_validator.h", "once_condition_validator.h",
"single_invalid_configuration.cc", "single_invalid_configuration.cc",
"single_invalid_configuration.h", "single_invalid_configuration.h",
"storage_validator.h",
"store.h", "store.h",
] ]
...@@ -69,10 +74,12 @@ source_set("unit_tests") { ...@@ -69,10 +74,12 @@ source_set("unit_tests") {
sources = [ sources = [
"chrome_variations_configuration_unittest.cc", "chrome_variations_configuration_unittest.cc",
"editable_configuration_unittest.cc", "editable_configuration_unittest.cc",
"feature_config_storage_validator_unittest.cc",
"feature_engagement_tracker_impl_unittest.cc", "feature_engagement_tracker_impl_unittest.cc",
"in_memory_store_unittest.cc", "in_memory_store_unittest.cc",
"model_impl_unittest.cc", "model_impl_unittest.cc",
"never_condition_validator_unittest.cc", "never_condition_validator_unittest.cc",
"never_storage_validator_unittest.cc",
"once_condition_validator_unittest.cc", "once_condition_validator_unittest.cc",
"single_invalid_configuration_unittest.cc", "single_invalid_configuration_unittest.cc",
] ]
......
...@@ -5,14 +5,17 @@ ...@@ -5,14 +5,17 @@
#ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_CONDITION_VALIDATOR_H_ #ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_CONDITION_VALIDATOR_H_
#define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_CONDITION_VALIDATOR_H_ #define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_CONDITION_VALIDATOR_H_
#include <string>
#include "base/macros.h" #include "base/macros.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/feature_list.h"
namespace base { namespace base {
struct Feature; struct Feature;
} // namespace base } // namespace base
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
class Model;
// A ConditionValidator checks the requred conditions for a given feature, // A ConditionValidator checks the requred conditions for a given feature,
// and checks if all conditions are met. // and checks if all conditions are met.
......
// 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 "components/feature_engagement_tracker/internal/feature_config_storage_validator.h"
#include <unordered_map>
#include <unordered_set>
#include "components/feature_engagement_tracker/internal/configuration.h"
#include "components/feature_engagement_tracker/internal/feature_list.h"
namespace feature_engagement_tracker {
FeatureConfigStorageValidator::FeatureConfigStorageValidator() = default;
FeatureConfigStorageValidator::~FeatureConfigStorageValidator() = default;
bool FeatureConfigStorageValidator::ShouldStore(const std::string& event_name) {
return should_store_event_names_.find(event_name) !=
should_store_event_names_.end();
}
bool FeatureConfigStorageValidator::ShouldKeep(const std::string& event_name,
uint32_t event_day,
uint32_t current_day) {
// Should not keep events that will happen in the future.
if (event_day > current_day)
return false;
// If no feature configuration mentioned the event, it should not be kept.
auto it = longest_storage_times_.find(event_name);
if (it == longest_storage_times_.end())
return false;
// Too old events should not be kept.
uint32_t longest_storage_time = it->second;
uint32_t age = current_day - event_day;
if (longest_storage_time <= age)
return false;
return true;
}
void FeatureConfigStorageValidator::InitializeFeatures(
FeatureVector features,
const Configuration& configuration) {
for (const auto* feature : features)
InitializeFeatureConfig(configuration.GetFeatureConfig(*feature));
}
void FeatureConfigStorageValidator::ClearForTesting() {
should_store_event_names_.clear();
longest_storage_times_.clear();
}
void FeatureConfigStorageValidator::InitializeFeatureConfig(
const FeatureConfig& feature_config) {
InitializeEventConfig(feature_config.used);
InitializeEventConfig(feature_config.trigger);
for (const auto& event_config : feature_config.event_configs)
InitializeEventConfig(event_config);
}
void FeatureConfigStorageValidator::InitializeEventConfig(
const EventConfig& event_config) {
// Minimum storage time is 1 day.
if (event_config.storage < 1u)
return;
// When minimum storage time is met, new events should always be stored.
should_store_event_names_.insert(event_config.name);
// Track the longest time any configuration wants to store a particular event.
uint32_t current_longest_time = longest_storage_times_[event_config.name];
if (event_config.storage > current_longest_time)
longest_storage_times_[event_config.name] = event_config.storage;
}
} // namespace feature_engagement_tracker
// 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.
#ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_FEATURE_CONFIG_STORAGE_VALIDATOR_H_
#define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_FEATURE_CONFIG_STORAGE_VALIDATOR_H_
#include <string>
#include <unordered_map>
#include <unordered_set>
#include "base/macros.h"
#include "components/feature_engagement_tracker/internal/feature_list.h"
#include "components/feature_engagement_tracker/internal/storage_validator.h"
namespace feature_engagement_tracker {
class Configuration;
struct EventConfig;
struct FeatureConfig;
// A StorageValidator that uses the FeatureConfiguration from
class FeatureConfigStorageValidator : public StorageValidator {
public:
FeatureConfigStorageValidator();
~FeatureConfigStorageValidator() override;
// ConditionValidator implementation.
bool ShouldStore(const std::string& event_name) override;
bool ShouldKeep(const std::string& event_name,
uint32_t event_day,
uint32_t current_day) override;
// Set up internal configuration required for the given |features|.
void InitializeFeatures(FeatureVector features,
const Configuration& configuration);
// Resets the full state of this StorageValidator. After calling this method
// it is valid to call InitializeFeatures() again.
void ClearForTesting();
private:
// Updates the internal configuration with conditions from the given
// |feature_config|.
void InitializeFeatureConfig(const FeatureConfig& feature_config);
// Updates the internal configuration with conditions from the given
// |event_config|.
void InitializeEventConfig(const EventConfig& event_config);
// Contains an entry for each of the events that any EventConfig required to
// be stored.
std::unordered_set<std::string> should_store_event_names_;
// Contains the longest time to store each event across all EventConfigs,
// as a number of days.
std::unordered_map<std::string, uint32_t> longest_storage_times_;
DISALLOW_COPY_AND_ASSIGN(FeatureConfigStorageValidator);
};
} // namespace feature_engagement_tracker
#endif // COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_FEATURE_CONFIG_STORAGE_VALIDATOR_H_
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "components/feature_engagement_tracker/internal/in_memory_store.h" #include "components/feature_engagement_tracker/internal/in_memory_store.h"
#include "components/feature_engagement_tracker/internal/model_impl.h" #include "components/feature_engagement_tracker/internal/model_impl.h"
#include "components/feature_engagement_tracker/internal/never_condition_validator.h" #include "components/feature_engagement_tracker/internal/never_condition_validator.h"
#include "components/feature_engagement_tracker/internal/never_storage_validator.h"
#include "components/feature_engagement_tracker/internal/once_condition_validator.h" #include "components/feature_engagement_tracker/internal/once_condition_validator.h"
#include "components/feature_engagement_tracker/internal/single_invalid_configuration.h" #include "components/feature_engagement_tracker/internal/single_invalid_configuration.h"
#include "components/feature_engagement_tracker/public/feature_constants.h" #include "components/feature_engagement_tracker/public/feature_constants.h"
...@@ -37,7 +38,8 @@ CreateDemoModeFeatureEngagementTracker() { ...@@ -37,7 +38,8 @@ CreateDemoModeFeatureEngagementTracker() {
return base::MakeUnique<FeatureEngagementTrackerImpl>( return base::MakeUnique<FeatureEngagementTrackerImpl>(
base::MakeUnique<InMemoryStore>(), std::move(configuration), base::MakeUnique<InMemoryStore>(), std::move(configuration),
base::MakeUnique<OnceConditionValidator>()); base::MakeUnique<OnceConditionValidator>(),
base::MakeUnique<NeverStorageValidator>());
} }
} // namespace } // namespace
...@@ -53,24 +55,27 @@ FeatureEngagementTracker* FeatureEngagementTracker::Create( ...@@ -53,24 +55,27 @@ FeatureEngagementTracker* FeatureEngagementTracker::Create(
return CreateDemoModeFeatureEngagementTracker().release(); return CreateDemoModeFeatureEngagementTracker().release();
std::unique_ptr<Store> store = base::MakeUnique<InMemoryStore>(); std::unique_ptr<Store> store = base::MakeUnique<InMemoryStore>();
// TODO(nyquist): Create FinchConfiguration to parse configuration.
std::unique_ptr<Configuration> configuration = std::unique_ptr<Configuration> configuration =
base::MakeUnique<SingleInvalidConfiguration>(); base::MakeUnique<SingleInvalidConfiguration>();
std::unique_ptr<ConditionValidator> validator = std::unique_ptr<ConditionValidator> condition_validator =
base::MakeUnique<NeverConditionValidator>(); base::MakeUnique<NeverConditionValidator>();
std::unique_ptr<StorageValidator> storage_validator =
base::MakeUnique<NeverStorageValidator>();
return new FeatureEngagementTrackerImpl( return new FeatureEngagementTrackerImpl(
std::move(store), std::move(configuration), std::move(validator)); std::move(store), std::move(configuration),
std::move(condition_validator), std::move(storage_validator));
} }
FeatureEngagementTrackerImpl::FeatureEngagementTrackerImpl( FeatureEngagementTrackerImpl::FeatureEngagementTrackerImpl(
std::unique_ptr<Store> store, std::unique_ptr<Store> store,
std::unique_ptr<Configuration> configuration, std::unique_ptr<Configuration> configuration,
std::unique_ptr<ConditionValidator> condition_validator) std::unique_ptr<ConditionValidator> condition_validator,
std::unique_ptr<StorageValidator> storage_validator)
: condition_validator_(std::move(condition_validator)), : condition_validator_(std::move(condition_validator)),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
model_ = model_ = base::MakeUnique<ModelImpl>(
base::MakeUnique<ModelImpl>(std::move(store), std::move(configuration)); std::move(store), std::move(configuration), std::move(storage_validator));
model_->Initialize( model_->Initialize(
base::Bind(&FeatureEngagementTrackerImpl::OnModelInitializationFinished, base::Bind(&FeatureEngagementTrackerImpl::OnModelInitializationFinished,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
......
...@@ -12,12 +12,14 @@ ...@@ -12,12 +12,14 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/supports_user_data.h" #include "base/supports_user_data.h"
#include "components/feature_engagement_tracker/internal/condition_validator.h"
#include "components/feature_engagement_tracker/internal/model.h"
#include "components/feature_engagement_tracker/public/feature_engagement_tracker.h" #include "components/feature_engagement_tracker/public/feature_engagement_tracker.h"
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
class Configuration;
class ConditionValidator;
class Model;
class Store; class Store;
class StorageValidator;
// The internal implementation of the FeatureEngagementTracker. // The internal implementation of the FeatureEngagementTracker.
class FeatureEngagementTrackerImpl : public FeatureEngagementTracker, class FeatureEngagementTrackerImpl : public FeatureEngagementTracker,
...@@ -26,7 +28,8 @@ class FeatureEngagementTrackerImpl : public FeatureEngagementTracker, ...@@ -26,7 +28,8 @@ class FeatureEngagementTrackerImpl : public FeatureEngagementTracker,
FeatureEngagementTrackerImpl( FeatureEngagementTrackerImpl(
std::unique_ptr<Store> store, std::unique_ptr<Store> store,
std::unique_ptr<Configuration> configuration, std::unique_ptr<Configuration> configuration,
std::unique_ptr<ConditionValidator> condition_validator); std::unique_ptr<ConditionValidator> condition_validator,
std::unique_ptr<StorageValidator> storage_validator);
~FeatureEngagementTrackerImpl() override; ~FeatureEngagementTrackerImpl() override;
// FeatureEngagementTracker implementation. // FeatureEngagementTracker implementation.
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "components/feature_engagement_tracker/internal/editable_configuration.h" #include "components/feature_engagement_tracker/internal/editable_configuration.h"
#include "components/feature_engagement_tracker/internal/in_memory_store.h" #include "components/feature_engagement_tracker/internal/in_memory_store.h"
#include "components/feature_engagement_tracker/internal/never_storage_validator.h"
#include "components/feature_engagement_tracker/internal/once_condition_validator.h" #include "components/feature_engagement_tracker/internal/once_condition_validator.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -42,8 +43,10 @@ class FeatureEngagementTrackerImplTest : public ::testing::Test { ...@@ -42,8 +43,10 @@ class FeatureEngagementTrackerImplTest : public ::testing::Test {
std::unique_ptr<Store> store = base::MakeUnique<InMemoryStore>(); std::unique_ptr<Store> store = base::MakeUnique<InMemoryStore>();
std::unique_ptr<EditableConfiguration> configuration = std::unique_ptr<EditableConfiguration> configuration =
base::MakeUnique<EditableConfiguration>(); base::MakeUnique<EditableConfiguration>();
std::unique_ptr<ConditionValidator> validator = std::unique_ptr<ConditionValidator> condition_validator =
base::MakeUnique<OnceConditionValidator>(); base::MakeUnique<OnceConditionValidator>();
std::unique_ptr<StorageValidator> storage_validator =
base::MakeUnique<NeverStorageValidator>();
RegisterFeatureConfig(configuration.get(), kTestFeatureFoo, true); RegisterFeatureConfig(configuration.get(), kTestFeatureFoo, true);
RegisterFeatureConfig(configuration.get(), kTestFeatureBar, true); RegisterFeatureConfig(configuration.get(), kTestFeatureBar, true);
...@@ -53,7 +56,8 @@ class FeatureEngagementTrackerImplTest : public ::testing::Test { ...@@ -53,7 +56,8 @@ class FeatureEngagementTrackerImplTest : public ::testing::Test {
{kTestFeatureFoo, kTestFeatureBar, kTestFeatureQux}, {}); {kTestFeatureFoo, kTestFeatureBar, kTestFeatureQux}, {});
tracker_.reset(new FeatureEngagementTrackerImpl( tracker_.reset(new FeatureEngagementTrackerImpl(
std::move(store), std::move(configuration), std::move(validator))); std::move(store), std::move(configuration),
std::move(condition_validator), std::move(storage_validator)));
// Ensure all initialization is finished. // Ensure all initialization is finished.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "components/feature_engagement_tracker/internal/configuration.h"
namespace base { namespace base {
struct Feature; struct Feature;
...@@ -18,6 +17,7 @@ struct Feature; ...@@ -18,6 +17,7 @@ struct Feature;
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
class Event; class Event;
struct FeatureConfig;
// A Model provides all necessary runtime state. // A Model provides all necessary runtime state.
class Model { class Model {
...@@ -55,6 +55,9 @@ class Model { ...@@ -55,6 +55,9 @@ class Model {
// If the event has never happened before, the Event object will be created. // If the event has never happened before, the Event object will be created.
virtual void IncrementEvent(const std::string& event_name) = 0; virtual void IncrementEvent(const std::string& event_name) = 0;
// Returns the number of days since epoch (1970-01-01) in the local timezone.
virtual uint32_t GetCurrentDay() = 0;
protected: protected:
Model() = default; Model() = default;
......
...@@ -16,15 +16,18 @@ ...@@ -16,15 +16,18 @@
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "components/feature_engagement_tracker/internal/configuration.h" #include "components/feature_engagement_tracker/internal/configuration.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/model.h"
#include "components/feature_engagement_tracker/internal/storage_validator.h"
#include "components/feature_engagement_tracker/internal/store.h" #include "components/feature_engagement_tracker/internal/store.h"
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
ModelImpl::ModelImpl(std::unique_ptr<Store> store, ModelImpl::ModelImpl(std::unique_ptr<Store> store,
std::unique_ptr<Configuration> configuration) std::unique_ptr<Configuration> configuration,
std::unique_ptr<StorageValidator> storage_validator)
: Model(), : Model(),
store_(std::move(store)), store_(std::move(store)),
configuration_(std::move(configuration)), configuration_(std::move(configuration)),
storage_validator_(std::move(storage_validator)),
ready_(false), ready_(false),
currently_showing_(false), currently_showing_(false),
weak_factory_(this) {} weak_factory_(this) {}
...@@ -61,6 +64,8 @@ void ModelImpl::IncrementEvent(const std::string& event_name) { ...@@ -61,6 +64,8 @@ void ModelImpl::IncrementEvent(const std::string& event_name) {
// TODO(nyquist): Add support for pending events, and also add UMA. // TODO(nyquist): Add support for pending events, and also add UMA.
DCHECK(ready_); DCHECK(ready_);
// TODO(nyquist): Use StorageValidator to check if the event should be stored.
Event& event = GetNonConstEvent(event_name); Event& event = GetNonConstEvent(event_name);
uint32_t current_day = GetCurrentDay(); uint32_t current_day = GetCurrentDay();
for (int i = 0; i < event.events_size(); ++i) { for (int i = 0; i < event.events_size(); ++i) {
...@@ -100,7 +105,7 @@ void ModelImpl::OnStoreLoaded(const OnModelInitializationFinished& callback, ...@@ -100,7 +105,7 @@ void ModelImpl::OnStoreLoaded(const OnModelInitializationFinished& callback,
events_[event.name()] = event; events_[event.name()] = event;
} }
// TODO(nyquist): Clear expired data. // TODO(nyquist): Use StorageValidator to only keep relevant event data.
ready_ = true; ready_ = true;
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "components/feature_engagement_tracker/internal/configuration.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/model.h"
#include "components/feature_engagement_tracker/internal/proto/event.pb.h" #include "components/feature_engagement_tracker/internal/proto/event.pb.h"
...@@ -21,13 +20,16 @@ struct Feature; ...@@ -21,13 +20,16 @@ struct Feature;
} }
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
class Configuration;
class StorageValidator;
class Store; class Store;
// A ModelImpl provides the default implementation of the Model. // A ModelImpl provides the default implementation of the Model.
class ModelImpl : public Model { class ModelImpl : public Model {
public: public:
ModelImpl(std::unique_ptr<Store> store, ModelImpl(std::unique_ptr<Store> store,
std::unique_ptr<Configuration> configuration); std::unique_ptr<Configuration> configuration,
std::unique_ptr<StorageValidator> storage_validator);
~ModelImpl() override; ~ModelImpl() override;
// Model implementation. // Model implementation.
...@@ -39,11 +41,7 @@ class ModelImpl : public Model { ...@@ -39,11 +41,7 @@ class ModelImpl : public Model {
bool IsCurrentlyShowing() const override; bool IsCurrentlyShowing() const override;
const Event& GetEvent(const std::string& event_name) override; const Event& GetEvent(const std::string& event_name) override;
void IncrementEvent(const std::string& event_name) override; void IncrementEvent(const std::string& event_name) override;
uint32_t GetCurrentDay() override;
protected:
// Returns the number of days since epoch (1970-01-01) in the local timezone.
// Protected and virtual for testing.
virtual uint32_t GetCurrentDay();
private: private:
// Callback for loading the underlying store. // Callback for loading the underlying store.
...@@ -61,6 +59,10 @@ class ModelImpl : public Model { ...@@ -61,6 +59,10 @@ class ModelImpl : public Model {
// The current configuration for all features. // The current configuration for all features.
std::unique_ptr<Configuration> configuration_; std::unique_ptr<Configuration> configuration_;
// A utility for checking whether new events should be stored and for whether
// old events should be kept.
std::unique_ptr<StorageValidator> storage_validator_;
// An in-memory representation of all events. // An in-memory representation of all events.
std::map<std::string, Event> events_; std::map<std::string, Event> events_;
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "components/feature_engagement_tracker/internal/editable_configuration.h" #include "components/feature_engagement_tracker/internal/editable_configuration.h"
#include "components/feature_engagement_tracker/internal/in_memory_store.h" #include "components/feature_engagement_tracker/internal/in_memory_store.h"
#include "components/feature_engagement_tracker/internal/never_storage_validator.h"
#include "components/feature_engagement_tracker/internal/proto/event.pb.h" #include "components/feature_engagement_tracker/internal/proto/event.pb.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -124,8 +125,11 @@ std::unique_ptr<TestInMemoryStore> CreatePrefilledStore() { ...@@ -124,8 +125,11 @@ std::unique_ptr<TestInMemoryStore> CreatePrefilledStore() {
class TestModelImpl : public ModelImpl { class TestModelImpl : public ModelImpl {
public: public:
TestModelImpl(std::unique_ptr<Store> store, TestModelImpl(std::unique_ptr<Store> store,
std::unique_ptr<Configuration> configuration) std::unique_ptr<Configuration> configuration,
: ModelImpl(std::move(store), std::move(configuration)), std::unique_ptr<StorageValidator> storage_validator)
: ModelImpl(std::move(store),
std::move(configuration),
std::move(storage_validator)),
current_day_(0) {} current_day_(0) {}
~TestModelImpl() override {} ~TestModelImpl() override {}
...@@ -155,7 +159,8 @@ class ModelImplTest : public ::testing::Test { ...@@ -155,7 +159,8 @@ class ModelImplTest : public ::testing::Test {
std::unique_ptr<TestInMemoryStore> store = CreateStore(); std::unique_ptr<TestInMemoryStore> store = CreateStore();
store_ = store.get(); store_ = store.get();
model_.reset(new TestModelImpl(std::move(store), std::move(configuration))); model_.reset(new TestModelImpl(std::move(store), std::move(configuration),
base::MakeUnique<NeverStorageValidator>()));
} }
virtual std::unique_ptr<TestInMemoryStore> CreateStore() { virtual std::unique_ptr<TestInMemoryStore> CreateStore() {
......
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
#include "components/feature_engagement_tracker/internal/never_condition_validator.h" #include "components/feature_engagement_tracker/internal/never_condition_validator.h"
#include "components/feature_engagement_tracker/internal/model.h"
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
NeverConditionValidator::NeverConditionValidator() = default; NeverConditionValidator::NeverConditionValidator() = default;
......
...@@ -5,17 +5,16 @@ ...@@ -5,17 +5,16 @@
#ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_CONDITION_VALIDATOR_H_ #ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_CONDITION_VALIDATOR_H_
#define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_CONDITION_VALIDATOR_H_ #define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_CONDITION_VALIDATOR_H_
#include <unordered_set>
#include "base/macros.h" #include "base/macros.h"
#include "components/feature_engagement_tracker/internal/condition_validator.h" #include "components/feature_engagement_tracker/internal/condition_validator.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/feature_list.h"
namespace base { namespace base {
struct Feature; struct Feature;
} // namespace base } // namespace base
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
class Model;
// An ConditionValidator that never acknowledges that a feature has met its // An ConditionValidator that never acknowledges that a feature has met its
// conditions. // conditions.
......
...@@ -4,9 +4,12 @@ ...@@ -4,9 +4,12 @@
#include "components/feature_engagement_tracker/internal/never_condition_validator.h" #include "components/feature_engagement_tracker/internal/never_condition_validator.h"
#include <string>
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "components/feature_engagement_tracker/internal/configuration.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/model.h"
#include "components/feature_engagement_tracker/internal/proto/event.pb.h" #include "components/feature_engagement_tracker/internal/proto/event.pb.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -47,6 +50,8 @@ class TestModel : public Model { ...@@ -47,6 +50,8 @@ class TestModel : public Model {
void IncrementEvent(const std::string& event_name) override {} void IncrementEvent(const std::string& event_name) override {}
uint32_t GetCurrentDay() override { return 0u; }
private: private:
FeatureConfig feature_config_; FeatureConfig feature_config_;
Event empty_event_; Event empty_event_;
......
// 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 "components/feature_engagement_tracker/internal/never_storage_validator.h"
namespace feature_engagement_tracker {
NeverStorageValidator::NeverStorageValidator() = default;
NeverStorageValidator::~NeverStorageValidator() = default;
bool NeverStorageValidator::ShouldStore(const std::string& event_name) {
return false;
}
bool NeverStorageValidator::ShouldKeep(const std::string& event_name,
uint32_t event_day,
uint32_t current_day) {
return false;
}
} // namespace feature_engagement_tracker
// 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.
#ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_STORAGE_VALIDATOR_H_
#define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_STORAGE_VALIDATOR_H_
#include <string>
#include "base/macros.h"
#include "components/feature_engagement_tracker/internal/storage_validator.h"
namespace feature_engagement_tracker {
// A StorageValidator that never acknowledges that an event should be kept or
// stored.
class NeverStorageValidator : public StorageValidator {
public:
NeverStorageValidator();
~NeverStorageValidator() override;
// StorageValidator implementation.
bool ShouldStore(const std::string& event_name) override;
bool ShouldKeep(const std::string& event_name,
uint32_t event_day,
uint32_t current_day) override;
private:
DISALLOW_COPY_AND_ASSIGN(NeverStorageValidator);
};
} // namespace feature_engagement_tracker
#endif // COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_NEVER_STORAGE_VALIDATOR_H_
// 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 "components/feature_engagement_tracker/internal/never_storage_validator.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace feature_engagement_tracker {
namespace {
class NeverStorageValidatorTest : public ::testing::Test {
public:
NeverStorageValidatorTest() = default;
protected:
NeverStorageValidator validator_;
private:
DISALLOW_COPY_AND_ASSIGN(NeverStorageValidatorTest);
};
} // namespace
TEST_F(NeverStorageValidatorTest, ShouldNeverKeep) {
EXPECT_FALSE(validator_.ShouldStore("dummy event"));
}
TEST_F(NeverStorageValidatorTest, ShouldNeverStore) {
EXPECT_FALSE(validator_.ShouldKeep("dummy event", 99, 100));
EXPECT_FALSE(validator_.ShouldKeep("dummy event", 100, 100));
EXPECT_FALSE(validator_.ShouldKeep("dummy event", 101, 100));
}
} // namespace feature_engagement_tracker
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
#include "components/feature_engagement_tracker/internal/once_condition_validator.h" #include "components/feature_engagement_tracker/internal/once_condition_validator.h"
#include "base/feature_list.h"
#include "components/feature_engagement_tracker/internal/configuration.h" #include "components/feature_engagement_tracker/internal/configuration.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/model.h"
......
...@@ -9,13 +9,14 @@ ...@@ -9,13 +9,14 @@
#include "base/macros.h" #include "base/macros.h"
#include "components/feature_engagement_tracker/internal/condition_validator.h" #include "components/feature_engagement_tracker/internal/condition_validator.h"
#include "components/feature_engagement_tracker/internal/model.h" #include "components/feature_engagement_tracker/internal/feature_list.h"
namespace base { namespace base {
struct Feature; struct Feature;
} // namespace base } // namespace base
namespace feature_engagement_tracker { namespace feature_engagement_tracker {
class Model;
// An ConditionValidator that will ensure that each base::Feature will meet // An ConditionValidator that will ensure that each base::Feature will meet
// conditions maximum one time for any given session. // conditions maximum one time for any given session.
......
...@@ -53,6 +53,8 @@ class TestModel : public Model { ...@@ -53,6 +53,8 @@ class TestModel : public Model {
void IncrementEvent(const std::string& event_name) override {} void IncrementEvent(const std::string& event_name) override {}
uint32_t GetCurrentDay() override { return 0u; }
private: private:
EditableConfiguration configuration_; EditableConfiguration configuration_;
Event empty_event_; Event empty_event_;
......
// 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.
#ifndef COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_STORAGE_VALIDATOR_H_
#define COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_STORAGE_VALIDATOR_H_
#include <string>
#include "base/macros.h"
namespace feature_engagement_tracker {
// A StorageValidator checks the required storage conditions for a given event,
// and checks if all conditions are met for storing it.
class StorageValidator {
public:
virtual ~StorageValidator() = default;
// Returns true iff new events of this type should be stored.
// This is typically called before storing each incoming event.
virtual bool ShouldStore(const std::string& event_name) = 0;
// Returns true iff events of this type should be kept for the given day.
// This is typically called during load of the internal database state, to
// possibly throw away old data.
virtual bool ShouldKeep(const std::string& event_name,
uint32_t event_day,
uint32_t current_day) = 0;
protected:
StorageValidator() = default;
private:
DISALLOW_COPY_AND_ASSIGN(StorageValidator);
};
} // namespace feature_engagement_tracker
#endif // COMPONENTS_FEATURE_ENGAGEMENT_TRACKER_INTERNAL_STORAGE_VALIDATOR_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment