[modular] Allow closing StoryPuppetMaster connections before Execute() has completely finished.
Some clients want to fire-and-forget the Execute() call. This allows
them to do it.
TEST=puppet_master_impl_unittest
Change-Id: I395bc8d5ab172597ed34a393e18efec2245e12bf
diff --git a/bin/sessionmgr/puppet_master/puppet_master_impl.cc b/bin/sessionmgr/puppet_master/puppet_master_impl.cc
index 68c466c..7d10e05 100644
--- a/bin/sessionmgr/puppet_master/puppet_master_impl.cc
+++ b/bin/sessionmgr/puppet_master/puppet_master_impl.cc
@@ -30,15 +30,13 @@
fidl::StringPtr story_name,
fidl::InterfaceRequest<fuchsia::modular::StoryPuppetMaster> request) {
auto controller = std::make_unique<StoryPuppetMasterImpl>(
- story_name, session_storage_, executor_);
+ story_name, &operations_, session_storage_, executor_);
story_puppet_masters_.AddBinding(std::move(controller), std::move(request));
}
void PuppetMasterImpl::DeleteStory(fidl::StringPtr story_name,
DeleteStoryCallback done) {
- session_storage_->DeleteStory(story_name)->Then([done = std::move(done)] {
- done();
- });
+ session_storage_->DeleteStory(story_name)->Then(std::move(done));
}
void PuppetMasterImpl::GetStories(GetStoriesCallback done) {
diff --git a/bin/sessionmgr/puppet_master/puppet_master_impl.h b/bin/sessionmgr/puppet_master/puppet_master_impl.h
index 6260956..d124bd9 100644
--- a/bin/sessionmgr/puppet_master/puppet_master_impl.h
+++ b/bin/sessionmgr/puppet_master/puppet_master_impl.h
@@ -8,6 +8,7 @@
#include <memory>
#include <fuchsia/modular/cpp/fidl.h>
+#include <lib/async/cpp/operation.h>
#include <lib/fidl/cpp/binding_set.h>
#include <lib/fxl/memory/weak_ptr.h>
@@ -51,6 +52,8 @@
std::unique_ptr<StoryPuppetMasterImpl>>
story_puppet_masters_;
+ OperationCollection operations_;
+
FXL_DISALLOW_COPY_AND_ASSIGN(PuppetMasterImpl);
};
diff --git a/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc b/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc
index 24176e3..5e1a963 100644
--- a/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc
+++ b/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc
@@ -74,10 +74,7 @@
EXPECT_EQ(1, executor_.execute_count());
EXPECT_EQ(fuchsia::modular::ExecuteStatus::OK, result.status);
- // Executor gets the internal story id. It is auto-generated, so we simply see
- // that we got one.
- EXPECT_TRUE(executor_.last_story_id() &&
- executor_.last_story_id()->size() > 0);
+ EXPECT_EQ("foo", executor_.last_story_id());
ASSERT_EQ(3u, executor_.last_commands().size());
EXPECT_EQ("one",
executor_.last_commands().at(0).remove_mod().mod_name->at(0));
@@ -87,6 +84,32 @@
executor_.last_commands().at(2).remove_mod().mod_name->at(0));
}
+TEST_F(PuppetMasterTest, CommandsAreSentToExecutor_IfWeCloseStoryChannel) {
+ // We're going to call Execute(), and then immediately drop the
+ // StoryPuppetMaster connection. We won't get a callback, but we still
+ // expect that the commands are executed.
+ auto story = ControlStory("foo");
+
+ // Enqueue some commands. Do this twice and show that all the commands show
+ // up as one batch.
+ fidl::VectorPtr<fuchsia::modular::StoryCommand> commands;
+ commands.push_back(MakeRemoveModCommand("one"));
+ story->Enqueue(std::move(commands));
+
+ fuchsia::modular::ExecuteResult result;
+ bool callback_called{false};
+ // Instruct our test executor to return an OK status.
+ executor_.SetExecuteReturnResult(fuchsia::modular::ExecuteStatus::OK,
+ nullptr);
+ story->Execute([&](fuchsia::modular::ExecuteResult r) {
+ callback_called = true;
+ });
+ story.Close(ZX_OK);
+ RunLoopUntil([&]() { return executor_.execute_count() > 0; });
+ EXPECT_FALSE(callback_called);
+ EXPECT_EQ(1, executor_.execute_count());
+}
+
TEST_F(PuppetMasterTest, MultipleExecuteCalls) {
// Create a new story, and then execute some new commands on the same
// connection. We should see that the StoryCommandExecutor receives the story
diff --git a/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc b/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc
index 8593aaf..588a5f4 100644
--- a/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc
+++ b/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc
@@ -75,11 +75,12 @@
} // namespace
StoryPuppetMasterImpl::StoryPuppetMasterImpl(
- fidl::StringPtr story_name, SessionStorage* const session_storage,
- StoryCommandExecutor* const executor)
+ fidl::StringPtr story_name, OperationContainer* const operations,
+ SessionStorage* const session_storage, StoryCommandExecutor* const executor)
: story_name_(story_name),
session_storage_(session_storage),
- executor_(executor) {
+ executor_(executor),
+ operations_(operations) {
FXL_DCHECK(session_storage != nullptr);
FXL_DCHECK(executor != nullptr);
}
@@ -98,9 +99,9 @@
void StoryPuppetMasterImpl::Execute(ExecuteCallback done) {
// First ensure that the story is created.
- operations_.Add(new ExecuteOperation(session_storage_, executor_, story_name_,
- std::move(story_options_),
- std::move(enqueued_commands_), done));
+ operations_->Add(new ExecuteOperation(session_storage_, executor_,
+ story_name_, std::move(story_options_),
+ std::move(enqueued_commands_), done));
}
void StoryPuppetMasterImpl::SetCreateOptions(
diff --git a/bin/sessionmgr/puppet_master/story_puppet_master_impl.h b/bin/sessionmgr/puppet_master/story_puppet_master_impl.h
index 49c6d93..40d3ecd 100644
--- a/bin/sessionmgr/puppet_master/story_puppet_master_impl.h
+++ b/bin/sessionmgr/puppet_master/story_puppet_master_impl.h
@@ -22,6 +22,7 @@
class StoryPuppetMasterImpl : public fuchsia::modular::StoryPuppetMaster {
public:
StoryPuppetMasterImpl(fidl::StringPtr story_name,
+ OperationContainer* operations,
SessionStorage* session_storage,
StoryCommandExecutor* executor);
~StoryPuppetMasterImpl() override;
@@ -43,7 +44,7 @@
std::vector<fuchsia::modular::StoryCommand> enqueued_commands_;
- OperationCollection operations_;
+ OperationContainer* const operations_; // Not owned.
fuchsia::modular::StoryOptions story_options_;
