Skip to content

Commit

Permalink
Remove UserDataManger and moves the methods to EngineInterface.
Browse files Browse the repository at this point in the history
Remove SetDataLoaderForTesting and SetAlwaysWaitForLoaderResponseFutureForTesting from EngineInterface.

PiperOrigin-RevId: 703336103
  • Loading branch information
taku910 authored and hiroyuki-komatsu committed Dec 6, 2024
1 parent dc30d0a commit 62fe473
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 303 deletions.
34 changes: 0 additions & 34 deletions src/engine/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,10 @@ mozc_cc_library(
"//base:thread",
"//data_manager",
"//protocol:engine_builder_cc_proto",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/log",
"@com_google_absl//absl/log:check",
"@com_google_absl//absl/status",
"@com_google_absl//absl/synchronization",
],
)

Expand Down Expand Up @@ -84,11 +82,8 @@ mozc_cc_library(
name = "engine_interface",
hdrs = ["engine_interface.h"],
deps = [
":engine_builder",
":supplemental_model_interface",
":user_data_manager_interface",
"//converter:converter_interface",
"//data_manager:data_manager_interface",
"//protocol:engine_builder_cc_proto",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
Expand Down Expand Up @@ -189,7 +184,6 @@ mozc_cc_library(
":minimal_engine",
":modules",
":supplemental_model_interface",
":user_data_manager_interface",
"//base:vlog",
"//converter",
"//converter:converter_interface",
Expand Down Expand Up @@ -241,9 +235,7 @@ mozc_cc_library(
deps = [
":engine_interface",
":supplemental_model_interface",
":user_data_manager_interface",
"//converter:converter_interface",
"//data_manager:data_manager_interface",
"//testing:gunit",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings:string_view",
Expand Down Expand Up @@ -358,31 +350,6 @@ mozc_cc_library(
],
)

mozc_cc_library(
name = "user_data_manager_mock",
testonly = 1,
hdrs = ["user_data_manager_mock.h"],
visibility = [
"//:__subpackages__",
],
deps = [
":user_data_manager_interface",
"//testing:gunit",
],
)

mozc_cc_library(
name = "user_data_manager_interface",
hdrs = ["user_data_manager_interface.h"],
visibility = [
# For //session:session_handler_scenario_test.
"//session:__pkg__",
],
deps = [
"@com_google_absl//absl/strings",
],
)

mozc_cc_library(
name = "minimal_engine",
srcs = ["minimal_engine.cc"],
Expand All @@ -393,7 +360,6 @@ mozc_cc_library(
deps = [
":engine_interface",
":modules",
":user_data_manager_interface",
"//base/strings:assign",
"//composer",
"//converter:converter_interface",
Expand Down
102 changes: 25 additions & 77 deletions src/engine/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
#include "engine/data_loader.h"
#include "engine/modules.h"
#include "engine/supplemental_model_interface.h"
#include "engine/user_data_manager_interface.h"
#include "prediction/dictionary_predictor.h"
#include "prediction/predictor.h"
#include "prediction/predictor_interface.h"
Expand All @@ -56,60 +55,6 @@
#include "rewriter/rewriter_interface.h"

namespace mozc {
namespace {

using ::mozc::prediction::PredictorInterface;

class UserDataManager final : public UserDataManagerInterface {
public:
UserDataManager(PredictorInterface *predictor, RewriterInterface *rewriter)
: predictor_(predictor), rewriter_(rewriter) {}

UserDataManager(const UserDataManager &) = delete;
UserDataManager &operator=(const UserDataManager &) = delete;

bool Sync() override;
bool Reload() override;
bool ClearUserHistory() override;
bool ClearUserPrediction() override;
bool ClearUnusedUserPrediction() override;
bool Wait() override;

private:
PredictorInterface *predictor_;
RewriterInterface *rewriter_;
};

bool UserDataManager::Sync() {
// TODO(noriyukit): In the current implementation, if rewriter_->Sync() fails,
// predictor_->Sync() is never called. Check if we should call
// predictor_->Sync() or not.
return rewriter_->Sync() && predictor_->Sync();
}

bool UserDataManager::Reload() {
// TODO(noriyukit): The same TODO as Sync().
return rewriter_->Reload() && predictor_->Reload();
}

bool UserDataManager::ClearUserHistory() {
rewriter_->Clear();
return true;
}

bool UserDataManager::ClearUserPrediction() {
predictor_->ClearAllHistory();
return true;
}

bool UserDataManager::ClearUnusedUserPrediction() {
predictor_->ClearUnusedHistory();
return true;
}

bool UserDataManager::Wait() { return predictor_->Wait(); }

} // namespace

absl::StatusOr<std::unique_ptr<Engine>> Engine::CreateDesktopEngine(
std::unique_ptr<const DataManagerInterface> data_manager) {
Expand Down Expand Up @@ -191,7 +136,7 @@ absl::Status Engine::Init(std::unique_ptr<engine::Modules> modules,
converter_ = std::make_unique<Converter>(*modules_, *immutable_converter_);
RETURN_IF_NULL(converter_);

std::unique_ptr<PredictorInterface> predictor;
std::unique_ptr<prediction::PredictorInterface> predictor;
{
// Create a predictor with three sub-predictors, dictionary predictor, user
// history predictor, and extra predictor.
Expand Down Expand Up @@ -225,45 +170,48 @@ absl::Status Engine::Init(std::unique_ptr<engine::Modules> modules,

converter_->Init(std::move(predictor), std::move(rewriter));

user_data_manager_ = std::make_unique<UserDataManager>(predictor_, rewriter_);

initialized_ = true;
return absl::Status();

#undef RETURN_IF_NULL
}

bool Engine::Reload() {
if (!modules_->GetUserDictionary()) {
return true;
if (modules_ && modules_->GetUserDictionary()) {
modules_->GetUserDictionary()->Reload();
}
MOZC_VLOG(1) << "Reloading user dictionary";
bool result_dictionary = modules_->GetUserDictionary()->Reload();
MOZC_VLOG(1) << "Reloading UserDataManager";
bool result_user_data = GetUserDataManager()->Reload();
return result_dictionary && result_user_data;
return rewriter_ && rewriter_->Reload() && predictor_ && predictor_->Reload();
}

bool Engine::Sync() {
GetUserDataManager()->Sync();
if (!modules_->GetUserDictionary()) {
return true;
if (modules_ && modules_->GetUserDictionary()) {
modules_->GetUserDictionary()->Sync();
}
return modules_->GetUserDictionary()->Sync();
return rewriter_ && rewriter_->Sync() && predictor_ && predictor_->Sync();
}

bool Engine::Wait() {
if (modules_->GetUserDictionary()) {
if (modules_ && modules_->GetUserDictionary()) {
modules_->GetUserDictionary()->WaitForReloader();
}
return GetUserDataManager()->Wait();
return predictor_ && predictor_->Wait();
}

bool Engine::ReloadAndWait() {
if (!Reload()) {
return false;
bool Engine::ReloadAndWait() { return Reload() && Wait(); }

bool Engine::ClearUserHistory() {
if (rewriter_) {
rewriter_->Clear();
}
return Wait();
return true;
}

bool Engine::ClearUserPrediction() {
return predictor_ && predictor_->ClearAllHistory();
}

bool Engine::ClearUnusedUserPrediction() {
return predictor_ && predictor_->ClearUnusedHistory();
}

bool Engine::MaybeReloadEngine(EngineReloadResponse *response) {
Expand Down Expand Up @@ -303,8 +251,8 @@ bool Engine::MaybeReloadEngine(EngineReloadResponse *response) {
continue;
}

if (user_data_manager_) {
user_data_manager_->Wait();
if (predictor_) {
predictor_->Wait();
}

// Reloads DataManager.
Expand Down
16 changes: 7 additions & 9 deletions src/engine/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
#include "engine/minimal_engine.h"
#include "engine/modules.h"
#include "engine/supplemental_model_interface.h"
#include "engine/user_data_manager_interface.h"
#include "prediction/predictor_interface.h"
#include "rewriter/rewriter_interface.h"

Expand Down Expand Up @@ -113,14 +112,13 @@ class Engine : public EngineInterface {
bool Wait() override;
bool ReloadAndWait() override;

bool ClearUserHistory() override;
bool ClearUserPrediction() override;
bool ClearUnusedUserPrediction() override;

absl::Status ReloadModules(std::unique_ptr<engine::Modules> modules,
bool is_mobile);

UserDataManagerInterface *GetUserDataManager() override {
return initialized_ ? user_data_manager_.get()
: minimal_engine_.GetUserDataManager();
}

absl::string_view GetDataVersion() const override {
return GetDataManager()->GetDataVersion();
}
Expand Down Expand Up @@ -150,10 +148,11 @@ class Engine : public EngineInterface {
// Maybe reload a new data manager. Returns true if reloaded.
bool MaybeReloadEngine(EngineReloadResponse *response) override;
bool SendEngineReloadRequest(const EngineReloadRequest &request) override;
void SetDataLoaderForTesting(std::unique_ptr<DataLoader> loader) override {

void SetDataLoaderForTesting(std::unique_ptr<DataLoader> loader) {
loader_ = std::move(loader);
}
void SetAlwaysWaitForLoaderResponseFutureForTesting(bool value) override {
void SetAlwaysWaitForLoaderResponseFutureForTesting(bool value) {
loader_->SetAlwaysWaitForLoaderResponseFutureForTesting(value);
}

Expand All @@ -179,7 +178,6 @@ class Engine : public EngineInterface {
RewriterInterface *rewriter_ = nullptr;

std::unique_ptr<Converter> converter_;
std::unique_ptr<UserDataManagerInterface> user_data_manager_;
};

} // namespace mozc
Expand Down
28 changes: 15 additions & 13 deletions src/engine/engine_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@
#include "absl/status/status.h"
#include "absl/strings/string_view.h"
#include "converter/converter_interface.h"
#include "engine/data_loader.h"
#include "engine/supplemental_model_interface.h"
#include "engine/user_data_manager_interface.h"
#include "protocol/engine_builder.pb.h"

namespace mozc {
Expand All @@ -62,32 +60,38 @@ class EngineInterface {
// Returns the predictor name.
virtual absl::string_view GetPredictorName() const = 0;

// Gets the version of underlying data set.
virtual absl::string_view GetDataVersion() const = 0;

// Reloads internal data, e.g., user dictionary, etc.
// This function may read data from local files.
// Returns true if successfully reloaded or did nothing.
virtual bool Reload() = 0;
virtual bool Reload() { return true; }

// Synchronizes internal data, e.g., user dictionary, etc.
// This function may write data into local files.
// Returns true if successfully synced or did nothing.
virtual bool Sync() = 0;
virtual bool Sync() { return true; }

// Waits for reloader.
// Returns true if successfully waited or did nothing.
virtual bool Wait() = 0;
virtual bool Wait() { return true; }

// Reloads internal data and wait for reloader.
// Returns true if successfully reloaded and waited, or did nothing.
virtual bool ReloadAndWait() = 0;
virtual bool ReloadAndWait() { return true; }

// Gets a user data manager.
virtual UserDataManagerInterface *GetUserDataManager() = 0;
// Clears user history data.
virtual bool ClearUserHistory() { return true; }

// Gets the version of underlying data set.
virtual absl::string_view GetDataVersion() const = 0;
// Clears user prediction data.
virtual bool ClearUserPrediction() { return true; }

// Clears unused user prediction data.
virtual bool ClearUnusedUserPrediction() { return true; }

// Gets the user POS list.
virtual std::vector<std::string> GetPosList() const = 0;
virtual std::vector<std::string> GetPosList() const { return {}; }

virtual void SetSupplementalModel(
const engine::SupplementalModelInterface *supplemental_model) {}
Expand All @@ -99,8 +103,6 @@ class EngineInterface {
virtual bool SendEngineReloadRequest(const EngineReloadRequest &request) {
return false;
}
virtual void SetDataLoaderForTesting(std::unique_ptr<DataLoader> loader) {}
virtual void SetAlwaysWaitForLoaderResponseFutureForTesting(bool value) {}

protected:
EngineInterface() = default;
Expand Down
7 changes: 4 additions & 3 deletions src/engine/engine_mock.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include "converter/converter_interface.h"
#include "engine/engine_interface.h"
#include "engine/supplemental_model_interface.h"
#include "engine/user_data_manager_interface.h"
#include "testing/gmock.h"

namespace mozc {
Expand All @@ -48,12 +47,14 @@ class MockEngine : public EngineInterface {
public:
MOCK_METHOD(ConverterInterface *, GetConverter, (), (const, override));
MOCK_METHOD(absl::string_view, GetPredictorName, (), (const, override));
MOCK_METHOD(absl::string_view, GetDataVersion, (), (const, override));
MOCK_METHOD(bool, Reload, (), (override));
MOCK_METHOD(bool, Sync, (), (override));
MOCK_METHOD(bool, Wait, (), (override));
MOCK_METHOD(bool, ClearUserHistory, (), (override));
MOCK_METHOD(bool, ClearUserPrediction, (), (override));
MOCK_METHOD(bool, ClearUnusedUserPrediction, (), (override));
MOCK_METHOD(bool, ReloadAndWait, (), (override));
MOCK_METHOD(UserDataManagerInterface *, GetUserDataManager, (), (override));
MOCK_METHOD(absl::string_view, GetDataVersion, (), (const, override));
MOCK_METHOD(std::vector<std::string>, GetPosList, (), (const, override));
MOCK_METHOD(void, SetSupplementalModel,
(const engine::SupplementalModelInterface *), (override));
Expand Down
Loading

0 comments on commit 62fe473

Please sign in to comment.