Skip to content

Commit

Permalink
Removed EngineBuilderInterface.
Browse files Browse the repository at this point in the history
We do not need to have an interface class if they are only used for
dependency injections and unittesting. Interface is used to provide
polymorphic behaviors.

PiperOrigin-RevId: 581810906
  • Loading branch information
taku910 authored and hiroyuki-komatsu committed Nov 13, 2023
1 parent b5e7def commit 96e76ef
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 187 deletions.
12 changes: 0 additions & 12 deletions src/engine/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,12 @@ package(
],
)

mozc_cc_library(
name = "engine_builder_interface",
hdrs = ["engine_builder_interface.h"],
deps = [
":engine_interface",
"//base:thread",
"//protocol:engine_builder_cc_proto",
"@com_google_absl//absl/strings",
],
)

mozc_cc_library(
name = "engine_builder",
srcs = ["engine_builder.cc"],
hdrs = ["engine_builder.h"],
deps = [
":engine",
":engine_builder_interface",
":engine_interface",
"//base:file_util",
"//base:hash",
Expand Down
2 changes: 2 additions & 0 deletions src/engine/engine.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
'../data_manager/oss/oss_data_manager.gyp:oss_data_manager',
'../prediction/prediction.gyp:prediction',
'engine',
'engine_builder'
],
},
{
Expand All @@ -106,6 +107,7 @@
'../dictionary/dictionary_base.gyp:pos_matcher',
'../request/request.gyp:conversion_request',
'engine',
'engine_builder'
],
},
{
Expand Down
37 changes: 31 additions & 6 deletions src/engine/engine_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,51 @@

#include "base/thread.h"
#include "data_manager/data_manager.h"
#include "engine/engine_builder_interface.h"
#include "engine/engine_interface.h"
#include "protocol/engine_builder.pb.h"
#include "absl/base/thread_annotations.h"
#include "absl/synchronization/mutex.h"

namespace mozc {

class EngineBuilder : public EngineBuilderInterface {
class EngineBuilder {
public:
EngineBuilder() = default;
EngineBuilder(const EngineBuilder &) = delete;
EngineBuilder &operator=(const EngineBuilder &) = delete;
~EngineBuilder() override = default;
virtual ~EngineBuilder() = default;

uint64_t RegisterRequest(const EngineReloadRequest &request) override;
struct EngineResponse {
uint64_t id = 0; // engine id. Fingerprint of EngineReloadRequest.
EngineReloadResponse response;
std::unique_ptr<EngineInterface> engine;
};

// Wrapped with BackgroundFuture so the data loading is
// executed asynchronously.
using EngineResponseFuture = BackgroundFuture<EngineResponse>;

// Accepts engine reload request and immediately returns the engine id with
// the highest priority defined as follows:
// - Request with higher request priority (e.g., downloaded > bundled)
// - When the priority is the same, the request registered last.
// The engine id 0 is reserved for unused engine.
virtual uint64_t RegisterRequest(const EngineReloadRequest &request);

uint64_t UnregisterRequest(uint64_t id) override;
// Unregister the request associated with the `id` and immediately returns
// the new engine id after the unregistration. This method is usually called
// to notify the request is not processed due to model loading failures and
// avoid the multiple loading operations. Client needs to load or use the
// engine of returned id. The unregistered request will not be accepted after
// calling this method.
virtual uint64_t UnregisterRequest(uint64_t id);

std::unique_ptr<EngineResponseFuture> Build(uint64_t id) const override;
// Builds the new engine associated with `id`.
// This method returns the future object immediately.
// Since BackgroundFuture is not movable/copyable, we wrap it with
// std::unique_ptr. This method doesn't return nullptr. All errors
// are stored in EngineReloadResponse::response::status.
virtual std::unique_ptr<EngineResponseFuture> Build(uint64_t id) const;

void Clear();

Expand Down
89 changes: 0 additions & 89 deletions src/engine/engine_builder_interface.h

This file was deleted.

6 changes: 3 additions & 3 deletions src/session/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ mozc_cc_library(
"//config:config_handler",
"//converter:converter_interface",
"//dictionary:user_dictionary_session_handler",
"//engine:engine_builder_interface",
"//engine:engine_builder",
"//engine:engine_interface",
"//engine:user_data_manager_interface",
"//protocol:commands_cc_proto",
Expand Down Expand Up @@ -377,7 +377,7 @@ mozc_cc_library(
"//config:config_handler",
"//converter:converter_interface",
"//dictionary:user_dictionary_session_handler",
"//engine:engine_builder_interface",
"//engine:engine_builder",
"//engine:engine_interface",
"//prediction:user_history_predictor",
"//protocol:commands_cc_proto",
Expand Down Expand Up @@ -415,7 +415,7 @@ mozc_cc_test(
"//config:config_handler",
"//converter:segments",
"//engine",
"//engine:engine_builder_interface",
"//engine:engine_builder",
"//engine:engine_interface",
"//engine:engine_mock",
"//engine:engine_stub",
Expand Down
14 changes: 6 additions & 8 deletions src/session/session_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
#include "config/character_form_manager.h"
#include "config/config_handler.h"
#include "dictionary/user_dictionary_session_handler.h"
#include "engine/engine_builder_interface.h"
#include "engine/engine_builder.h"
#include "engine/engine_interface.h"
#include "engine/user_data_manager_interface.h"
#include "protocol/commands.pb.h"
Expand Down Expand Up @@ -133,18 +133,16 @@ bool IsApplicationAlive(const session::SessionInterface *session) {
} // namespace

SessionHandler::SessionHandler(std::unique_ptr<EngineInterface> engine) {
Init(std::move(engine), std::unique_ptr<EngineBuilderInterface>());
Init(std::move(engine), std::unique_ptr<EngineBuilder>());
}

SessionHandler::SessionHandler(
std::unique_ptr<EngineInterface> engine,
std::unique_ptr<EngineBuilderInterface> engine_builder) {
SessionHandler::SessionHandler(std::unique_ptr<EngineInterface> engine,
std::unique_ptr<EngineBuilder> engine_builder) {
Init(std::move(engine), std::move(engine_builder));
}

void SessionHandler::Init(
std::unique_ptr<EngineInterface> engine,
std::unique_ptr<EngineBuilderInterface> engine_builder) {
void SessionHandler::Init(std::unique_ptr<EngineInterface> engine,
std::unique_ptr<EngineBuilder> engine_builder) {
is_available_ = false;
max_session_size_ = 0;
last_session_empty_time_ = Clock::GetAbslTime();
Expand Down
11 changes: 5 additions & 6 deletions src/session/session_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

#include "composer/table.h"
#include "dictionary/user_dictionary_session_handler.h"
#include "engine/engine_builder_interface.h"
#include "engine/engine_builder.h"
#include "engine/engine_interface.h"
#include "protocol/commands.pb.h"
#include "protocol/config.pb.h"
Expand All @@ -65,7 +65,7 @@ class SessionHandler : public SessionHandlerInterface {
public:
explicit SessionHandler(std::unique_ptr<EngineInterface> engine);
SessionHandler(std::unique_ptr<EngineInterface> engine,
std::unique_ptr<EngineBuilderInterface> engine_builder);
std::unique_ptr<EngineBuilder> engine_builder);
SessionHandler(const SessionHandler &) = delete;
SessionHandler &operator=(const SessionHandler &) = delete;
~SessionHandler() override;
Expand Down Expand Up @@ -99,7 +99,7 @@ class SessionHandler : public SessionHandlerInterface {
using SessionElement = SessionMap::Element;

void Init(std::unique_ptr<EngineInterface> engine,
std::unique_ptr<EngineBuilderInterface> engine_builder);
std::unique_ptr<EngineBuilder> engine_builder);

// Updates the config, if the |command| contains the config.
void MaybeUpdateConfig(commands::Command *command);
Expand Down Expand Up @@ -154,16 +154,15 @@ class SessionHandler : public SessionHandlerInterface {
absl::Time last_create_session_time_ = absl::InfinitePast();

std::unique_ptr<EngineInterface> engine_;
std::unique_ptr<EngineBuilderInterface> engine_builder_;
std::unique_ptr<EngineBuilder> engine_builder_;
std::unique_ptr<session::SessionObserverHandler> observer_handler_;
std::unique_ptr<user_dictionary::UserDictionarySessionHandler>
user_dictionary_session_handler_;
std::unique_ptr<composer::TableManager> table_manager_;
std::unique_ptr<const commands::Request> request_;
std::unique_ptr<const config::Config> config_;
std::unique_ptr<keymap::KeyMapManager> key_map_manager_;
std::unique_ptr<EngineBuilderInterface::EngineResponseFuture>
engine_response_future_;
std::unique_ptr<EngineBuilder::EngineResponseFuture> engine_response_future_;

// used only in unittest to perform blocking behavior.
bool always_wait_for_engine_response_future_ = false;
Expand Down
Loading

0 comments on commit 96e76ef

Please sign in to comment.