Commit f410cc4c authored by Dan Harrington's avatar Dan Harrington Committed by Commit Bot

Check for stale content on load

- Updated code so content fetch time is saved
- If content is stale, load from the network.

Bug: 1044139
Change-Id: I676ac6dab77133815286b14d93b94b5b778b3386
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2131012Reviewed-by: default avatarIan Wells <iwells@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755459}
parent e865a301
...@@ -47,7 +47,7 @@ class UserClassifier { ...@@ -47,7 +47,7 @@ class UserClassifier {
// The provided |pref_service| may be nullptr in unit-tests. // The provided |pref_service| may be nullptr in unit-tests.
UserClassifier(PrefService* pref_service, const base::Clock* clock); UserClassifier(PrefService* pref_service, const base::Clock* clock);
~UserClassifier(); virtual ~UserClassifier();
// Registers profile prefs for all rates. Called from pref_names.cc. // Registers profile prefs for all rates. Called from pref_names.cc.
static void RegisterProfilePrefs(PrefRegistrySimple* registry); static void RegisterProfilePrefs(PrefRegistrySimple* registry);
...@@ -61,7 +61,8 @@ class UserClassifier { ...@@ -61,7 +61,8 @@ class UserClassifier {
double GetEstimatedAvgTime(Event event) const; double GetEstimatedAvgTime(Event event) const;
// Return the classification of the current user. // Return the classification of the current user.
UserClass GetUserClass() const; // Virtual for testing.
virtual UserClass GetUserClass() const;
std::string GetUserClassDescriptionForDebugging() const; std::string GetUserClassDescriptionForDebugging() const;
// Resets the classification (emulates a fresh upgrade / install). // Resets the classification (emulates a fresh upgrade / install).
......
...@@ -29,6 +29,10 @@ std::ostream& operator<<(std::ostream& out, LoadStreamStatus value) { ...@@ -29,6 +29,10 @@ std::ostream& operator<<(std::ostream& out, LoadStreamStatus value) {
return out << "kNoResponseBody"; return out << "kNoResponseBody";
case LoadStreamStatus::kProtoTranslationFailed: case LoadStreamStatus::kProtoTranslationFailed:
return out << "kProtoTranslationFailed"; return out << "kProtoTranslationFailed";
case LoadStreamStatus::kDataInStoreIsStale:
return out << "kDataInStoreIsStale";
case LoadStreamStatus::kDataInStoreIsStaleTimestampInFuture:
return out << "kDataInStoreIsStaleTimestampInFuture";
} }
#else #else
return out << (static_cast<int>(value)); return out << (static_cast<int>(value));
......
...@@ -11,19 +11,6 @@ ...@@ -11,19 +11,6 @@
namespace feed { namespace feed {
// Describes the behavior for attempting to refresh (over the network) while
// loading the feed.
enum class LoadRefreshBehavior {
// Wait for feed refresh before showing the result.
kWaitForRefresh,
// Load what is available locally, begin the refresh, and populate results
// below the fold when they are received.
kRefreshInline,
// Wait a limited amount of time for the network fetch. If the fetch doesn't
// complete in time, just show the user what's available locally.
kLimitedWaitForRefresh,
};
enum class LoadStreamStatus { enum class LoadStreamStatus {
// Loading was not attempted. // Loading was not attempted.
kNoStatus = 0, kNoStatus = 0,
...@@ -35,6 +22,10 @@ enum class LoadStreamStatus { ...@@ -35,6 +22,10 @@ enum class LoadStreamStatus {
kNoResponseBody = 6, kNoResponseBody = 6,
// TODO(harringtond): Let's add more specific errors here. // TODO(harringtond): Let's add more specific errors here.
kProtoTranslationFailed = 7, kProtoTranslationFailed = 7,
kDataInStoreIsStale = 8,
// The timestamp for stored data is in the future, so we're treating stored
// data as it it is stale.
kDataInStoreIsStaleTimestampInFuture = 9,
}; };
std::ostream& operator<<(std::ostream& out, LoadStreamStatus value); std::ostream& operator<<(std::ostream& out, LoadStreamStatus value);
......
...@@ -132,8 +132,10 @@ class FeedStream::ModelMonitor : public StreamModel::Observer { ...@@ -132,8 +132,10 @@ class FeedStream::ModelMonitor : public StreamModel::Observer {
std::unique_ptr<StreamModelUpdateRequest> std::unique_ptr<StreamModelUpdateRequest>
FeedStream::WireResponseTranslator::TranslateWireResponse( FeedStream::WireResponseTranslator::TranslateWireResponse(
feedwire::Response response, feedwire::Response response,
base::TimeDelta response_time) { base::TimeDelta response_time,
return ::feed::TranslateWireResponse(std::move(response), response_time); base::Time current_time) {
return ::feed::TranslateWireResponse(std::move(response), response_time,
current_time);
} }
FeedStream::FeedStream( FeedStream::FeedStream(
...@@ -156,7 +158,7 @@ FeedStream::FeedStream( ...@@ -156,7 +158,7 @@ FeedStream::FeedStream(
tick_clock_(tick_clock), tick_clock_(tick_clock),
background_task_runner_(background_task_runner), background_task_runner_(background_task_runner),
task_queue_(this), task_queue_(this),
user_classifier_(profile_prefs, clock), user_classifier_(std::make_unique<UserClassifier>(profile_prefs, clock)),
refresh_throttler_(profile_prefs, clock) { refresh_throttler_(profile_prefs, clock) {
// TODO(harringtond): Use these members. // TODO(harringtond): Use these members.
static WireResponseTranslator default_translator; static WireResponseTranslator default_translator;
...@@ -248,7 +250,7 @@ bool FeedStream::RejectEphemeralChange(EphemeralChangeId id) { ...@@ -248,7 +250,7 @@ bool FeedStream::RejectEphemeralChange(EphemeralChangeId id) {
} }
UserClass FeedStream::GetUserClass() { UserClass FeedStream::GetUserClass() {
return user_classifier_.GetUserClass(); return user_classifier_->GetUserClass();
} }
base::Time FeedStream::GetLastFetchTime() { base::Time FeedStream::GetLastFetchTime() {
...@@ -277,6 +279,11 @@ void FeedStream::SetIdleCallbackForTesting( ...@@ -277,6 +279,11 @@ void FeedStream::SetIdleCallbackForTesting(
idle_callback_ = idle_callback; idle_callback_ = idle_callback;
} }
void FeedStream::SetUserClassifierForTesting(
std::unique_ptr<UserClassifier> user_classifier) {
user_classifier_ = std::move(user_classifier);
}
void FeedStream::OnStoreChange(const StreamModel::StoreUpdate& update) { void FeedStream::OnStoreChange(const StreamModel::StoreUpdate& update) {
store_->WriteOperations(update.sequence_number, update.operations); store_->WriteOperations(update.sequence_number, update.operations);
} }
......
...@@ -68,7 +68,8 @@ class FeedStream : public FeedStreamApi, ...@@ -68,7 +68,8 @@ class FeedStream : public FeedStreamApi,
~WireResponseTranslator() = default; ~WireResponseTranslator() = default;
virtual std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse( virtual std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse(
feedwire::Response response, feedwire::Response response,
base::TimeDelta response_time); base::TimeDelta response_time,
base::Time current_time);
}; };
FeedStream(RefreshTaskScheduler* refresh_task_scheduler, FeedStream(RefreshTaskScheduler* refresh_task_scheduler,
...@@ -149,6 +150,8 @@ class FeedStream : public FeedStreamApi, ...@@ -149,6 +150,8 @@ class FeedStream : public FeedStreamApi,
// Returns the model if it is loaded, or null otherwise. // Returns the model if it is loaded, or null otherwise.
StreamModel* GetModel() { return model_.get(); } StreamModel* GetModel() { return model_.get(); }
const base::Clock* GetClock() { return clock_; }
WireResponseTranslator* GetWireResponseTranslator() const { WireResponseTranslator* GetWireResponseTranslator() const {
return wire_response_translator_; return wire_response_translator_;
} }
...@@ -159,6 +162,8 @@ class FeedStream : public FeedStreamApi, ...@@ -159,6 +162,8 @@ class FeedStream : public FeedStreamApi,
} }
void SetIdleCallbackForTesting(base::RepeatingClosure idle_callback); void SetIdleCallbackForTesting(base::RepeatingClosure idle_callback);
void SetUserClassifierForTesting(
std::unique_ptr<UserClassifier> user_classifier);
private: private:
class ModelMonitor; class ModelMonitor;
...@@ -206,7 +211,7 @@ class FeedStream : public FeedStreamApi, ...@@ -206,7 +211,7 @@ class FeedStream : public FeedStreamApi,
base::ObserverList<SurfaceInterface> surfaces_; base::ObserverList<SurfaceInterface> surfaces_;
// Mutable state. // Mutable state.
UserClassifier user_classifier_; std::unique_ptr<UserClassifier> user_classifier_;
MasterRefreshThrottler refresh_throttler_; MasterRefreshThrottler refresh_throttler_;
base::TimeTicks suppress_refreshes_until_; base::TimeTicks suppress_refreshes_until_;
......
...@@ -40,8 +40,13 @@ std::unique_ptr<StreamModel> LoadModelFromStore(FeedStore* store) { ...@@ -40,8 +40,13 @@ std::unique_ptr<StreamModel> LoadModelFromStore(FeedStore* store) {
auto complete = [&](LoadStreamFromStoreTask::Result task_result) { auto complete = [&](LoadStreamFromStoreTask::Result task_result) {
result = std::move(task_result); result = std::move(task_result);
}; };
LoadStreamFromStoreTask load_task(store, LoadStreamFromStoreTask load_task(
base::BindLambdaForTesting(complete)); store, /*clock=*/nullptr,
UserClass::kActiveSuggestionsConsumer, // Has no effect.
base::BindLambdaForTesting(complete));
// We want to load the data no matter how stale.
load_task.IgnoreStalenessForTesting();
base::RunLoop run_loop; base::RunLoop run_loop;
load_task.Execute(run_loop.QuitClosure()); load_task.Execute(run_loop.QuitClosure());
run_loop.Run(); run_loop.Run();
...@@ -107,6 +112,24 @@ class TestSurface : public FeedStream::SurfaceInterface { ...@@ -107,6 +112,24 @@ class TestSurface : public FeedStream::SurfaceInterface {
base::Optional<feedui::StreamUpdate> update; base::Optional<feedui::StreamUpdate> update;
}; };
class TestUserClassifier : public UserClassifier {
public:
TestUserClassifier(PrefService* pref_service, const base::Clock* clock)
: UserClassifier(pref_service, clock) {}
// UserClassifier.
UserClass GetUserClass() const override {
return overridden_user_class_.value_or(UserClassifier::GetUserClass());
}
// Test use.
void OverrideUserClass(UserClass user_class) {
overridden_user_class_ = user_class;
}
private:
base::Optional<UserClass> overridden_user_class_;
};
class TestFeedNetwork : public FeedNetwork { class TestFeedNetwork : public FeedNetwork {
public: public:
// FeedNetwork implementation. // FeedNetwork implementation.
...@@ -141,12 +164,13 @@ class TestWireResponseTranslator : public FeedStream::WireResponseTranslator { ...@@ -141,12 +164,13 @@ class TestWireResponseTranslator : public FeedStream::WireResponseTranslator {
public: public:
std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse( std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse(
feedwire::Response response, feedwire::Response response,
base::TimeDelta response_time) override { base::TimeDelta response_time,
base::Time current_time) override {
if (injected_response_) { if (injected_response_) {
return std::move(injected_response_); return std::move(injected_response_);
} }
return FeedStream::WireResponseTranslator::TranslateWireResponse( return FeedStream::WireResponseTranslator::TranslateWireResponse(
std::move(response), response_time); std::move(response), response_time, current_time);
} }
void InjectResponse(std::unique_ptr<StreamModelUpdateRequest> response) { void InjectResponse(std::unique_ptr<StreamModelUpdateRequest> response) {
injected_response_ = std::move(response); injected_response_ = std::move(response);
...@@ -193,12 +217,19 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate { ...@@ -193,12 +217,19 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
void SetUp() override { void SetUp() override {
feed::prefs::RegisterFeedSharedProfilePrefs(profile_prefs_.registry()); feed::prefs::RegisterFeedSharedProfilePrefs(profile_prefs_.registry());
feed::RegisterProfilePrefs(profile_prefs_.registry()); feed::RegisterProfilePrefs(profile_prefs_.registry());
CHECK_EQ(kTestTimeEpoch, task_environment_.GetMockClock()->Now());
stream_ = std::make_unique<FeedStream>( stream_ = std::make_unique<FeedStream>(
&refresh_scheduler_, &event_observer_, this, &profile_prefs_, &network_, &refresh_scheduler_, &event_observer_, this, &profile_prefs_, &network_,
store_.get(), &clock_, &tick_clock_, store_.get(), task_environment_.GetMockClock(),
task_environment_.GetMockTickClock(),
task_environment_.GetMainThreadTaskRunner()); task_environment_.GetMainThreadTaskRunner());
// Set the user classifier.
auto user_classifier = std::make_unique<TestUserClassifier>(
&profile_prefs_, task_environment_.GetMockClock());
user_classifier_ = user_classifier.get();
stream_->SetUserClassifierForTesting(std::move(user_classifier));
WaitForIdleTaskQueue(); // Wait for any initialization. WaitForIdleTaskQueue(); // Wait for any initialization.
stream_->SetWireResponseTranslatorForTesting(&response_translator_); stream_->SetWireResponseTranslatorForTesting(&response_translator_);
...@@ -242,6 +273,7 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate { ...@@ -242,6 +273,7 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
protected: protected:
base::test::TaskEnvironment task_environment_{ base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME}; base::test::TaskEnvironment::TimeSource::MOCK_TIME};
TestUserClassifier* user_classifier_;
TestEventObserver event_observer_; TestEventObserver event_observer_;
TestingPrefServiceSimple profile_prefs_; TestingPrefServiceSimple profile_prefs_;
TestFeedNetwork network_; TestFeedNetwork network_;
...@@ -252,8 +284,6 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate { ...@@ -252,8 +284,6 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
leveldb_proto::ProtoDbType::FEED_STREAM_DATABASE, leveldb_proto::ProtoDbType::FEED_STREAM_DATABASE,
/*file_path=*/{}, /*file_path=*/{},
task_environment_.GetMainThreadTaskRunner())); task_environment_.GetMainThreadTaskRunner()));
base::SimpleTestClock clock_;
base::SimpleTestTickClock tick_clock_;
FakeRefreshTaskScheduler refresh_scheduler_; FakeRefreshTaskScheduler refresh_scheduler_;
std::unique_ptr<FeedStream> stream_; std::unique_ptr<FeedStream> stream_;
}; };
...@@ -446,11 +476,40 @@ TEST_F(FeedStreamTest, LoadFromNetwork) { ...@@ -446,11 +476,40 @@ TEST_F(FeedStreamTest, LoadFromNetwork) {
// Verify the model is filled correctly. // Verify the model is filled correctly.
EXPECT_STRINGS_EQUAL(ModelStateFor(MakeTypicalInitialModelState()), EXPECT_STRINGS_EQUAL(ModelStateFor(MakeTypicalInitialModelState()),
stream_->GetModel()->DumpStateForTesting()); stream_->GetModel()->DumpStateForTesting());
// Verify the data was written to the store.
EXPECT_STRINGS_EQUAL(ModelStateFor(store_.get()),
ModelStateFor(MakeTypicalInitialModelState()));
}
TEST_F(FeedStreamTest, LoadFromNetworkBecauseStoreIsStale) {
// Fill the store with stream data that is just barely stale, and verify we
// fetch new data over the network.
user_classifier_->OverrideUserClass(UserClass::kActiveSuggestionsConsumer);
store_->SaveFullStream(MakeTypicalInitialModelState(
kTestTimeEpoch - base::TimeDelta::FromHours(12) -
base::TimeDelta::FromMinutes(1)),
base::DoNothing());
// Store is stale, so we should fallback to a network request.
response_translator_.InjectResponse(MakeTypicalInitialModelState());
TestSurface surface;
stream_->AttachSurface(&surface);
WaitForIdleTaskQueue();
EXPECT_TRUE(network_.query_request_sent);
EXPECT_TRUE(response_translator_.InjectedResponseConsumed());
ASSERT_TRUE(surface.initial_state);
} }
TEST_F(FeedStreamTest, LoadStreamFromStore) { TEST_F(FeedStreamTest, LoadStreamFromStore) {
// Fill the store with stream data, and verify it loads. // Fill the store with stream data that is just barely fresh, and verify it
store_->SaveFullStream(MakeTypicalInitialModelState(), base::DoNothing()); // loads.
user_classifier_->OverrideUserClass(UserClass::kActiveSuggestionsConsumer);
store_->SaveFullStream(MakeTypicalInitialModelState(
kTestTimeEpoch - base::TimeDelta::FromHours(12) +
base::TimeDelta::FromMinutes(1)),
base::DoNothing());
TestSurface surface; TestSurface surface;
stream_->AttachSurface(&surface); stream_->AttachSurface(&surface);
WaitForIdleTaskQueue(); WaitForIdleTaskQueue();
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "components/feed/core/proto/v2/store.pb.h"
namespace feed { namespace feed {
...@@ -34,3 +35,15 @@ bool CompareContentId(const feedwire::ContentId& a, ...@@ -34,3 +35,15 @@ bool CompareContentId(const feedwire::ContentId& a,
} }
} // namespace feed } // namespace feed
namespace feedstore {
void SetLastAddedTime(base::Time t, feedstore::StreamData* data) {
data->set_last_added_time_millis(
(t - base::Time::UnixEpoch()).InMilliseconds());
}
base::Time GetLastAddedTime(const feedstore::StreamData& data) {
return base::Time::UnixEpoch() +
base::TimeDelta::FromMilliseconds(data.last_added_time_millis());
}
} // namespace feedstore
...@@ -7,8 +7,14 @@ ...@@ -7,8 +7,14 @@
#include <string> #include <string>
#include "base/time/time.h"
#include "components/feed/core/proto/v2/wire/content_id.pb.h" #include "components/feed/core/proto/v2/wire/content_id.pb.h"
namespace feedstore {
class StreamData;
}
// Helper functions/classes for dealing with feed proto messages. // Helper functions/classes for dealing with feed proto messages.
namespace feed { namespace feed {
...@@ -28,4 +34,11 @@ class ContentIdCompareFunctor { ...@@ -28,4 +34,11 @@ class ContentIdCompareFunctor {
} // namespace feed } // namespace feed
namespace feedstore {
void SetLastAddedTime(base::Time t, feedstore::StreamData* data);
base::Time GetLastAddedTime(const feedstore::StreamData& data);
} // namespace feedstore
#endif // COMPONENTS_FEED_CORE_V2_PROTO_UTIL_H_ #endif // COMPONENTS_FEED_CORE_V2_PROTO_UTIL_H_
...@@ -40,16 +40,12 @@ base::TimeDelta GetUserClassTriggerThreshold(UserClass user_class, ...@@ -40,16 +40,12 @@ base::TimeDelta GetUserClassTriggerThreshold(UserClass user_class,
} }
} }
LoadRefreshBehavior DetermineLoadRefreshBehavior(UserClass user_class, bool ShouldWaitForNewContent(UserClass user_class,
bool has_content, bool has_content,
base::TimeDelta content_age) { base::TimeDelta content_age) {
if (!has_content) return !has_content ||
return LoadRefreshBehavior::kWaitForRefresh; content_age > GetUserClassTriggerThreshold(user_class,
if (content_age > TriggerType::kForegrounded);
GetUserClassTriggerThreshold(user_class, TriggerType::kForegrounded))
return LoadRefreshBehavior::kLimitedWaitForRefresh;
// TODO(harringtond): We are probably not going to support |kRefreshInline|.
return LoadRefreshBehavior::kRefreshInline;
} }
} // namespace feed } // namespace feed
...@@ -16,16 +16,15 @@ constexpr base::TimeDelta kSuppressRefreshDuration = ...@@ -16,16 +16,15 @@ constexpr base::TimeDelta kSuppressRefreshDuration =
// The following should be true: // The following should be true:
// - At most one fetch is attempted per T. // - At most one fetch is attempted per T.
// - Content is considered stale if time since last fetch is > T. We'll prefer // - Content is considered stale if time since last fetch is > T. We'll prefer
// to refresh stale content before showing it. See LoadRefreshBehavior. // to refresh stale content before showing it.
// - For TriggerType::kFixedTimer, T is the time between scheduled fetches. // - For TriggerType::kFixedTimer, T is the time between scheduled fetches.
base::TimeDelta GetUserClassTriggerThreshold(UserClass user_class, base::TimeDelta GetUserClassTriggerThreshold(UserClass user_class,
TriggerType trigger); TriggerType trigger);
// Determines which LoadRefreshBehavior should be used when refreshing the // Returns whether we should wait for new content before showing stream content.
// stream. bool ShouldWaitForNewContent(UserClass user_class,
LoadRefreshBehavior DetermineLoadRefreshBehavior(UserClass user_class, bool has_content,
bool has_content, base::TimeDelta content_age);
base::TimeDelta content_age);
} // namespace feed } // namespace feed
#endif // COMPONENTS_FEED_CORE_V2_SCHEDULING_H_ #endif // COMPONENTS_FEED_CORE_V2_SCHEDULING_H_
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "components/feed/core/proto/v2/wire/payload_metadata.pb.h" #include "components/feed/core/proto/v2/wire/payload_metadata.pb.h"
#include "components/feed/core/proto/v2/wire/stream_structure.pb.h" #include "components/feed/core/proto/v2/wire/stream_structure.pb.h"
#include "components/feed/core/proto/v2/wire/token.pb.h" #include "components/feed/core/proto/v2/wire/token.pb.h"
#include "components/feed/core/v2/proto_util.h"
namespace feed { namespace feed {
...@@ -208,7 +209,8 @@ base::Optional<feedstore::DataOperation> TranslateDataOperation( ...@@ -208,7 +209,8 @@ base::Optional<feedstore::DataOperation> TranslateDataOperation(
std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse( std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse(
feedwire::Response response, feedwire::Response response,
base::TimeDelta response_time) { base::TimeDelta response_time,
base::Time current_time) {
if (response.response_version() != feedwire::Response::FEED_RESPONSE) if (response.response_version() != feedwire::Response::FEED_RESPONSE)
return nullptr; return nullptr;
...@@ -242,10 +244,11 @@ std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse( ...@@ -242,10 +244,11 @@ std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse(
*result->stream_data.mutable_shared_state_id() = *result->stream_data.mutable_shared_state_id() =
result->shared_states.front().content_id(); result->shared_states.front().content_id();
} }
feedstore::SetLastAddedTime(current_time, &result->stream_data);
result->server_response_time = result->server_response_time =
feed_response->feed_response_metadata().response_time_ms(); feed_response->feed_response_metadata().response_time_ms();
result->response_time = response_time; result->response_time = response_time;
return result; return result;
} }
......
...@@ -66,7 +66,8 @@ base::Optional<feedstore::DataOperation> TranslateDataOperation( ...@@ -66,7 +66,8 @@ base::Optional<feedstore::DataOperation> TranslateDataOperation(
std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse( std::unique_ptr<StreamModelUpdateRequest> TranslateWireResponse(
feedwire::Response response, feedwire::Response response,
base::TimeDelta response_time); base::TimeDelta response_time,
base::Time current_time);
} // namespace feed } // namespace feed
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "components/feed/core/proto/v2/wire/feed_response.pb.h" #include "components/feed/core/proto/v2/wire/feed_response.pb.h"
#include "components/feed/core/proto/v2/wire/response.pb.h" #include "components/feed/core/proto/v2/wire/response.pb.h"
#include "components/feed/core/v2/proto_util.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace feed { namespace feed {
...@@ -21,7 +22,8 @@ namespace { ...@@ -21,7 +22,8 @@ namespace {
const char kResponsePbPath[] = "components/test/data/feed/response.binarypb"; const char kResponsePbPath[] = "components/test/data/feed/response.binarypb";
constexpr base::TimeDelta kResponseTime = base::TimeDelta::FromSeconds(42); constexpr base::TimeDelta kResponseTime = base::TimeDelta::FromSeconds(42);
const base::Time kCurrentTime =
base::Time::UnixEpoch() + base::TimeDelta::FromDays(123);
// TODO(iwells): Replace response.binarypb with a response that uses the new // TODO(iwells): Replace response.binarypb with a response that uses the new
// wire protocol. // wire protocol.
// //
...@@ -75,9 +77,10 @@ TEST(StreamModelUpdateRequestTest, TranslateRealResponse) { ...@@ -75,9 +77,10 @@ TEST(StreamModelUpdateRequestTest, TranslateRealResponse) {
kExpectedStreamStructureCount + 1); kExpectedStreamStructureCount + 1);
std::unique_ptr<StreamModelUpdateRequest> translated = std::unique_ptr<StreamModelUpdateRequest> translated =
TranslateWireResponse(response, kResponseTime); TranslateWireResponse(response, kResponseTime, kCurrentTime);
ASSERT_TRUE(translated); ASSERT_TRUE(translated);
EXPECT_EQ(kCurrentTime, feedstore::GetLastAddedTime(translated->stream_data));
ASSERT_EQ(translated->stream_structures.size(), ASSERT_EQ(translated->stream_structures.size(),
static_cast<size_t>(kExpectedStreamStructureCount)); static_cast<size_t>(kExpectedStreamStructureCount));
......
...@@ -7,9 +7,12 @@ ...@@ -7,9 +7,12 @@
#include <algorithm> #include <algorithm>
#include <utility> #include <utility>
#include "base/time/clock.h"
#include "components/feed/core/proto/v2/store.pb.h" #include "components/feed/core/proto/v2/store.pb.h"
#include "components/feed/core/v2/feed_store.h" #include "components/feed/core/v2/feed_store.h"
#include "components/feed/core/v2/proto_util.h"
#include "components/feed/core/v2/public/feed_stream_api.h" #include "components/feed/core/v2/public/feed_stream_api.h"
#include "components/feed/core/v2/scheduling.h"
#include "components/feed/core/v2/stream_model_update_request.h" #include "components/feed/core/v2/stream_model_update_request.h"
namespace feed { namespace feed {
...@@ -22,8 +25,12 @@ LoadStreamFromStoreTask::Result& LoadStreamFromStoreTask::Result::operator=( ...@@ -22,8 +25,12 @@ LoadStreamFromStoreTask::Result& LoadStreamFromStoreTask::Result::operator=(
LoadStreamFromStoreTask::LoadStreamFromStoreTask( LoadStreamFromStoreTask::LoadStreamFromStoreTask(
FeedStore* store, FeedStore* store,
const base::Clock* clock,
UserClass user_class,
base::OnceCallback<void(Result)> callback) base::OnceCallback<void(Result)> callback)
: store_(store), : store_(store),
clock_(clock),
user_class_(user_class),
result_callback_(std::move(callback)), result_callback_(std::move(callback)),
update_request_(std::make_unique<StreamModelUpdateRequest>()) {} update_request_(std::make_unique<StreamModelUpdateRequest>()) {}
...@@ -44,8 +51,19 @@ void LoadStreamFromStoreTask::LoadStreamDone( ...@@ -44,8 +51,19 @@ void LoadStreamFromStoreTask::LoadStreamDone(
Complete(LoadStreamStatus::kNoStreamDataInStore); Complete(LoadStreamStatus::kNoStreamDataInStore);
return; return;
} }
// TODO(harringtond): Add other failure cases: if (!ignore_staleness_) {
// - Is the content stale? const base::TimeDelta content_age =
clock_->Now() - feedstore::GetLastAddedTime(result.stream_data);
if (content_age < base::TimeDelta()) {
Complete(LoadStreamStatus::kDataInStoreIsStaleTimestampInFuture);
return;
} else if (ShouldWaitForNewContent(user_class_, true, content_age)) {
Complete(LoadStreamStatus::kDataInStoreIsStale);
return;
}
}
// TODO(harringtond): Add other failure cases?
std::vector<ContentId> referenced_content_ids; std::vector<ContentId> referenced_content_ids;
for (const feedstore::StreamStructureSet& structure_set : for (const feedstore::StreamStructureSet& structure_set :
......
...@@ -14,6 +14,10 @@ ...@@ -14,6 +14,10 @@
#include "components/feed/core/v2/feed_store.h" #include "components/feed/core/v2/feed_store.h"
#include "components/offline_pages/task/task.h" #include "components/offline_pages/task/task.h"
namespace base {
class Clock;
}
namespace feed { namespace feed {
struct StreamModelUpdateRequest; struct StreamModelUpdateRequest;
...@@ -30,11 +34,15 @@ class LoadStreamFromStoreTask : public offline_pages::Task { ...@@ -30,11 +34,15 @@ class LoadStreamFromStoreTask : public offline_pages::Task {
}; };
LoadStreamFromStoreTask(FeedStore* store, LoadStreamFromStoreTask(FeedStore* store,
const base::Clock* clock,
UserClass user_class,
base::OnceCallback<void(Result)> callback); base::OnceCallback<void(Result)> callback);
~LoadStreamFromStoreTask() override; ~LoadStreamFromStoreTask() override;
LoadStreamFromStoreTask(const LoadStreamFromStoreTask&) = delete; LoadStreamFromStoreTask(const LoadStreamFromStoreTask&) = delete;
LoadStreamFromStoreTask& operator=(const LoadStreamFromStoreTask&) = delete; LoadStreamFromStoreTask& operator=(const LoadStreamFromStoreTask&) = delete;
void IgnoreStalenessForTesting() { ignore_staleness_ = true; }
private: private:
void Run() override; void Run() override;
...@@ -48,6 +56,9 @@ class LoadStreamFromStoreTask : public offline_pages::Task { ...@@ -48,6 +56,9 @@ class LoadStreamFromStoreTask : public offline_pages::Task {
} }
FeedStore* store_; // Unowned. FeedStore* store_; // Unowned.
const base::Clock* clock_;
UserClass user_class_;
bool ignore_staleness_ = false;
base::OnceCallback<void(Result)> result_callback_; base::OnceCallback<void(Result)> result_callback_;
std::unique_ptr<StreamModelUpdateRequest> update_request_; std::unique_ptr<StreamModelUpdateRequest> update_request_;
......
...@@ -7,7 +7,9 @@ ...@@ -7,7 +7,9 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "base/bind_helpers.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/time/clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/feed/core/proto/v2/wire/client_info.pb.h" #include "components/feed/core/proto/v2/wire/client_info.pb.h"
#include "components/feed/core/proto/v2/wire/feed_request.pb.h" #include "components/feed/core/proto/v2/wire/feed_request.pb.h"
...@@ -37,7 +39,7 @@ void LoadStreamTask::Run() { ...@@ -37,7 +39,7 @@ void LoadStreamTask::Run() {
} }
load_from_store_task_ = std::make_unique<LoadStreamFromStoreTask>( load_from_store_task_ = std::make_unique<LoadStreamFromStoreTask>(
stream_->GetStore(), stream_->GetStore(), stream_->GetClock(), stream_->GetUserClass(),
base::BindOnce(&LoadStreamTask::LoadFromStoreComplete, GetWeakPtr())); base::BindOnce(&LoadStreamTask::LoadFromStoreComplete, GetWeakPtr()));
load_from_store_task_->Execute(base::DoNothing()); load_from_store_task_->Execute(base::DoNothing());
} }
...@@ -83,12 +85,17 @@ void LoadStreamTask::QueryRequestComplete( ...@@ -83,12 +85,17 @@ void LoadStreamTask::QueryRequestComplete(
std::unique_ptr<StreamModelUpdateRequest> update_request = std::unique_ptr<StreamModelUpdateRequest> update_request =
stream_->GetWireResponseTranslator()->TranslateWireResponse( stream_->GetWireResponseTranslator()->TranslateWireResponse(
*result.response_body, base::TimeTicks::Now() - fetch_start_time_); *result.response_body, base::TimeTicks::Now() - fetch_start_time_,
stream_->GetClock()->Now());
if (!update_request) { if (!update_request) {
Done(LoadStreamStatus::kProtoTranslationFailed); Done(LoadStreamStatus::kProtoTranslationFailed);
return; return;
} }
stream_->GetStore()->SaveFullStream(
std::make_unique<StreamModelUpdateRequest>(*update_request),
base::DoNothing());
auto model = std::make_unique<StreamModel>(); auto model = std::make_unique<StreamModel>();
model->Update(std::move(update_request)); model->Update(std::move(update_request));
stream_->LoadModel(std::move(model)); stream_->LoadModel(std::move(model));
......
...@@ -7,10 +7,13 @@ ...@@ -7,10 +7,13 @@
#include <utility> #include <utility>
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "components/feed/core/v2/proto_util.h"
#include "components/feed/core/v2/stream_model_update_request.h" #include "components/feed/core/v2/stream_model_update_request.h"
namespace feed { namespace feed {
const base::Time kTestTimeEpoch = base::Time::UnixEpoch();
ContentId MakeContentId(ContentId::Type type, ContentId MakeContentId(ContentId::Type type,
std::string content_domain, std::string content_domain,
int id_number) { int id_number) {
...@@ -143,7 +146,8 @@ std::vector<feedstore::DataOperation> MakeTypicalStreamOperations() { ...@@ -143,7 +146,8 @@ std::vector<feedstore::DataOperation> MakeTypicalStreamOperations() {
}; };
} }
std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState() { std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState(
base::Time last_added_time) {
auto initial_update = std::make_unique<StreamModelUpdateRequest>(); auto initial_update = std::make_unique<StreamModelUpdateRequest>();
initial_update->source = initial_update->source =
StreamModelUpdateRequest::Source::kInitialLoadFromStore; StreamModelUpdateRequest::Source::kInitialLoadFromStore;
...@@ -159,6 +163,7 @@ std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState() { ...@@ -159,6 +163,7 @@ std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState() {
initial_update->shared_states.push_back(MakeSharedState(0)); initial_update->shared_states.push_back(MakeSharedState(0));
*initial_update->stream_data.mutable_content_id() = MakeRootId(); *initial_update->stream_data.mutable_content_id() = MakeRootId();
*initial_update->stream_data.mutable_shared_state_id() = MakeSharedStateId(0); *initial_update->stream_data.mutable_shared_state_id() = MakeSharedStateId(0);
SetLastAddedTime(last_added_time, &initial_update->stream_data);
return initial_update; return initial_update;
} }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/time/time.h"
#include "components/feed/core/proto/v2/store.pb.h" #include "components/feed/core/proto/v2/store.pb.h"
#include "components/feed/core/v2/public/feed_stream_api.h" #include "components/feed/core/v2/public/feed_stream_api.h"
...@@ -16,6 +17,8 @@ ...@@ -16,6 +17,8 @@
namespace feed { namespace feed {
struct StreamModelUpdateRequest; struct StreamModelUpdateRequest;
extern const base::Time kTestTimeEpoch;
ContentId MakeContentId(ContentId::Type type, ContentId MakeContentId(ContentId::Type type,
std::string content_domain, std::string content_domain,
int id_number); int id_number);
...@@ -46,7 +49,8 @@ feedstore::Record MakeRecord(feedstore::StreamData stream_data); ...@@ -46,7 +49,8 @@ feedstore::Record MakeRecord(feedstore::StreamData stream_data);
// |-Cluster 1 // |-Cluster 1
// |-Content 1 // |-Content 1
std::vector<feedstore::DataOperation> MakeTypicalStreamOperations(); std::vector<feedstore::DataOperation> MakeTypicalStreamOperations();
std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState(); std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState(
base::Time last_added_time = kTestTimeEpoch);
} // namespace feed } // namespace feed
#endif // COMPONENTS_FEED_CORE_V2_TEST_STREAM_BUILDER_H_ #endif // COMPONENTS_FEED_CORE_V2_TEST_STREAM_BUILDER_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