[modular] Use StoryInfo2 in StoryData; make SetStoryInfoExtra a no-op
This is part of a soft-transition of StoryInfo from struct to table.
SetStoryInfoExtra is not being used and will be superceded by annotations.
Bug: 16407
Change-Id: Iefd751e91dced797c1f51e4a5401c65933829978
diff --git a/peridot/bin/sessionmgr/puppet_master/puppet_master_impl.cc b/peridot/bin/sessionmgr/puppet_master/puppet_master_impl.cc
index d0a31fe..b1dbff7 100644
--- a/peridot/bin/sessionmgr/puppet_master/puppet_master_impl.cc
+++ b/peridot/bin/sessionmgr/puppet_master/puppet_master_impl.cc
@@ -37,10 +37,13 @@
void PuppetMasterImpl::GetStories(GetStoriesCallback done) {
session_storage_->GetAllStoryData()->Then(
- [done = std::move(done)](std::vector<fuchsia::modular::internal::StoryData> all_story_data) {
+ [done = std::move(done)](
+ const std::vector<fuchsia::modular::internal::StoryData>& all_story_data) {
std::vector<std::string> result;
+ result.reserve(all_story_data.size());
+
for (auto& story : all_story_data) {
- result.push_back(std::move(story.story_info().id));
+ result.push_back(story.story_info().id());
}
done(std::move(result));
diff --git a/peridot/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc b/peridot/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc
index 0fea8cb..95eb34e 100644
--- a/peridot/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc
+++ b/peridot/bin/sessionmgr/puppet_master/puppet_master_impl_unittest.cc
@@ -282,70 +282,6 @@
RunLoopUntil([&] { return done; });
}
-// Verifies that the StoryInfo extra field is set when calling SetStoryInfoExtra
-// prior to creating the story and executing story commands
-TEST_F(PuppetMasterTest, CreateStoryWithStoryInfoExtra) {
- const auto story_name = "story_info_extra_1";
- const auto extra_entry_key = "test_key";
- const auto extra_entry_value = "test_value";
-
- auto story = ControlStory(story_name);
-
- std::vector<fuchsia::modular::StoryInfoExtraEntry> extra_info{
- fuchsia::modular::StoryInfoExtraEntry{
- .key = extra_entry_key,
- .value = extra_entry_value,
- }};
- const auto extra_info_size = extra_info.size(); // For convenience, to avoid a wordy cast to
- // size_type in ASSERT_EQ below.
-
- // Try to SetStoryInfoExtra. It should succeed because the story has not been
- // created yet.
- bool done{};
- story->SetStoryInfoExtra(
- std::move(extra_info),
- [&](fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Result result) {
- EXPECT_FALSE(result.is_err());
- done = true;
- });
- RunLoopUntil([&] { return done; });
-
- // Enqueue some commands.
- std::vector<fuchsia::modular::StoryCommand> commands;
- commands.push_back(MakeRemoveModCommand("one"));
- story->Enqueue(std::move(commands));
-
- // The story, and its StoryData, does not exist until the story is created,
- // which is after the commands are executed.
- done = false;
- storage_->GetStoryData(story_name)->Then([&](fuchsia::modular::internal::StoryDataPtr data) {
- EXPECT_EQ(nullptr, data);
- done = true;
- });
- RunLoopUntil([&] { return done; });
-
- // Execute the commands, implicitly creating the story.
- done = false;
- story->Execute([&](fuchsia::modular::ExecuteResult result) {
- EXPECT_EQ(fuchsia::modular::ExecuteStatus::OK, result.status);
- done = true;
- });
- RunLoopUntil([&] { return done; });
-
- // StoryData should contain the StoryInfo extra value that was set previously
- done = false;
- storage_->GetStoryData(story_name)->Then([&](fuchsia::modular::internal::StoryDataPtr data) {
- ASSERT_NE(nullptr, data);
- ASSERT_TRUE(data->story_info().extra.has_value());
- auto extra_info = data->story_info().extra.value();
- ASSERT_EQ(extra_info.size(), extra_info_size);
- EXPECT_EQ(extra_info.at(0).key, extra_entry_key);
- EXPECT_EQ(extra_info.at(0).value, extra_entry_value);
- done = true;
- });
- RunLoopUntil([&] { return done; });
-}
-
// Verifies that calls to SetStoryInfoExtra after the story is created
// do not modify the original value.
TEST_F(PuppetMasterTest, SetStoryInfoExtraAfterCreateStory) {
@@ -384,41 +320,16 @@
.value = "ignored_value",
}};
- // Try to SetStoryInfoExtra. It should return an error because the story has
- // already been created.
+ // Try to SetStoryInfoExtra. It should not return an error even though the story has
+ // already been created, since the method is a no-op.
done = false;
story->SetStoryInfoExtra(
std::move(extra_info),
[&](fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Result result) {
- EXPECT_TRUE(result.is_err());
- EXPECT_EQ(result.err(), fuchsia::modular::ConfigureStoryError::ERR_STORY_ALREADY_CREATED);
+ EXPECT_FALSE(result.is_err());
done = true;
});
RunLoopUntil([&] { return done; });
-
- // Enqueue and execute some commands.
- std::vector<fuchsia::modular::StoryCommand> commands2;
- commands2.push_back(MakeRemoveModCommand("two"));
- story->Enqueue(std::move(commands2));
-
- done = false;
- story->Execute([&](fuchsia::modular::ExecuteResult result) {
- EXPECT_EQ(fuchsia::modular::ExecuteStatus::OK, result.status);
- done = true;
- });
- RunLoopUntil([&] { return done; });
-
- // Ensure the commands have been executed on the story created earlier.
- EXPECT_EQ(story_id, executor_.last_story_id());
-
- // StoryInfo extra should be null because it was not set prior
- // to creating the story.
- done = false;
- storage_->GetStoryData(story_name)->Then([&](fuchsia::modular::internal::StoryDataPtr data) {
- ASSERT_FALSE(data->story_info().extra.has_value());
- done = true;
- });
- RunLoopUntil([&] { return done; });
}
// Verifies that calls to SetStoryInfoExtra succeed after a story is deleted.
@@ -427,7 +338,7 @@
// Create the story.
bool done{};
- storage_->CreateStory(story_name, /* extra_info= */ {}, /* story_options= */ {})
+ storage_->CreateStory(story_name, /*story_options=*/{})
->Then([&](fidl::StringPtr id, fuchsia::ledger::PageId page_id) { done = true; });
RunLoopUntil([&] { return done; });
@@ -439,13 +350,12 @@
.value = "ignored_value",
}};
- // Try to SetStoryInfoExtra. It should return an error because the story has
- // already been created.
+ // Try to SetStoryInfoExtra. It should not return an error even though the story has
+ // already been created, since the method is a no-op.
done = false;
story->SetStoryInfoExtra(
extra_info, [&](fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Result result) {
- EXPECT_TRUE(result.is_err());
- EXPECT_EQ(result.err(), fuchsia::modular::ConfigureStoryError::ERR_STORY_ALREADY_CREATED);
+ EXPECT_FALSE(result.is_err());
done = true;
});
RunLoopUntil([&] { return done; });
@@ -470,8 +380,9 @@
std::string story_id;
// Create a story.
- storage_->CreateStory("foo", {} /* extra_info */, {} /* story_options */)
- ->Then([&](fidl::StringPtr id, fuchsia::ledger::PageId page_id) { story_id = id.value_or(""); });
+ storage_->CreateStory("foo", /*story_options=*/{})
+ ->Then(
+ [&](fidl::StringPtr id, fuchsia::ledger::PageId page_id) { story_id = id.value_or(""); });
// Delete it
bool done{};
@@ -497,7 +408,7 @@
RunLoopUntil([&] { return done; });
// Create a story.
- storage_->CreateStory("foo", {} /* extra_info */, {} /* story_options */);
+ storage_->CreateStory("foo", /*story_options=*/{});
// "foo" should be listed.
done = false;
diff --git a/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc b/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc
index 619717d..bb61465 100644
--- a/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc
+++ b/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.cc
@@ -5,6 +5,7 @@
#include "peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.h"
#include <lib/fsl/types/type_converters.h>
+
#include <src/lib/fxl/logging.h>
#include "peridot/bin/sessionmgr/puppet_master/story_command_executor.h"
@@ -18,14 +19,12 @@
public:
ExecuteOperation(SessionStorage* const session_storage, StoryCommandExecutor* const executor,
std::string story_name, fuchsia::modular::StoryOptions story_options,
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info_,
std::vector<fuchsia::modular::StoryCommand> commands, ResultCall done)
: Operation("StoryPuppetMasterImpl.ExecuteOperation", std::move(done)),
session_storage_(session_storage),
executor_(executor),
story_name_(std::move(story_name)),
story_options_(std::move(story_options)),
- extra_info_(std::move(extra_info_)),
commands_(std::move(commands)) {}
private:
@@ -33,7 +32,7 @@
session_storage_->GetStoryData(story_name_)
->WeakThen(GetWeakPtr(), [this](fuchsia::modular::internal::StoryDataPtr data) {
if (data) {
- story_id_ = data->story_info().id;
+ story_id_ = data->story_info().id();
ExecuteCommands();
return;
}
@@ -43,7 +42,7 @@
}
void CreateStory() {
- session_storage_->CreateStory(story_name_, std::move(extra_info_), std::move(story_options_))
+ session_storage_->CreateStory(story_name_, std::move(story_options_))
->WeakThen(GetWeakPtr(), [this](fidl::StringPtr story_id, auto /* ignored */) {
story_id_ = story_id;
ExecuteCommands();
@@ -62,7 +61,6 @@
StoryCommandExecutor* const executor_;
std::string story_name_;
fuchsia::modular::StoryOptions story_options_;
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info_;
std::vector<fuchsia::modular::StoryCommand> commands_;
fidl::StringPtr story_id_;
@@ -78,7 +76,6 @@
session_storage_(session_storage),
executor_(executor),
operations_(operations),
- story_info_extra_(nullptr),
weak_ptr_factory_(this) {
FXL_DCHECK(session_storage != nullptr);
FXL_DCHECK(executor != nullptr);
@@ -94,7 +91,7 @@
void StoryPuppetMasterImpl::Execute(ExecuteCallback done) {
operations_->Add(std::make_unique<ExecuteOperation>(
session_storage_, executor_, story_name_, std::move(story_options_),
- std::move(story_info_extra_), std::move(enqueued_commands_), std::move(done)));
+ std::move(enqueued_commands_), std::move(done)));
}
void StoryPuppetMasterImpl::SetCreateOptions(fuchsia::modular::StoryOptions story_options) {
@@ -104,26 +101,11 @@
void StoryPuppetMasterImpl::SetStoryInfoExtra(
std::vector<fuchsia::modular::StoryInfoExtraEntry> story_info_extra,
SetStoryInfoExtraCallback callback) {
- session_storage_->GetStoryData(story_name_)
- ->WeakThen(
- weak_ptr_factory_.GetWeakPtr(),
- [this, story_info_extra = std::move(story_info_extra),
- callback = std::move(callback)](fuchsia::modular::internal::StoryDataPtr story_data) {
- fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Result result;
- fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Response response;
-
- // StoryInfo can only be set before a story is created, and StoryData
- // does not exist until it has been created.
- if (!story_data) {
- story_info_extra_ = fidl::To<fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry>>(
- std::move(story_info_extra));
- result.set_response(response);
- } else {
- result.set_err(fuchsia::modular::ConfigureStoryError::ERR_STORY_ALREADY_CREATED);
- }
-
- callback(std::move(result));
- });
+ // This method is a no-op.
+ fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Result result{};
+ fuchsia::modular::StoryPuppetMaster_SetStoryInfoExtra_Response response{};
+ result.set_response(response);
+ callback(std::move(result));
}
} // namespace modular
diff --git a/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.h b/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.h
index 9a8fb83..4089172 100644
--- a/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.h
+++ b/peridot/bin/sessionmgr/puppet_master/story_puppet_master_impl.h
@@ -8,10 +8,11 @@
#include <fuchsia/modular/cpp/fidl.h>
#include <lib/async/cpp/operation.h>
#include <lib/fidl/cpp/binding_set.h>
-#include <src/lib/fxl/memory/weak_ptr.h>
#include <memory>
+#include <src/lib/fxl/memory/weak_ptr.h>
+
#include "peridot/bin/sessionmgr/puppet_master/story_command_executor.h"
namespace modular {
@@ -54,11 +55,6 @@
// in the first call to |Execute|, and subsequent values are ignored.
fuchsia::modular::StoryOptions story_options_;
- // StoryInfo extra entries passed to |session_storage_.CreateStory|, set
- // by |SetStoryInfoExtra|. This value is reset after the story is created
- // in the first call to |Execute|, and subsequent values are ignored.
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> story_info_extra_;
-
fxl::WeakPtrFactory<StoryPuppetMasterImpl> weak_ptr_factory_;
FXL_DISALLOW_COPY_AND_ASSIGN(StoryPuppetMasterImpl);
diff --git a/peridot/bin/sessionmgr/storage/session_storage.cc b/peridot/bin/sessionmgr/storage/session_storage.cc
index 84d1566..84659c8 100644
--- a/peridot/bin/sessionmgr/storage/session_storage.cc
+++ b/peridot/bin/sessionmgr/storage/session_storage.cc
@@ -57,20 +57,18 @@
fuchsia::ledger::Page* const page, fuchsia::modular::internal::StoryDataPtr story_data,
fit::function<void()> result_call) {
return std::make_unique<WriteDataCall<fuchsia::modular::internal::StoryData>>(
- page, StoryNameToStoryDataKey(story_data->story_info().id), XdrStoryData,
+ page, StoryNameToStoryDataKey(story_data->story_info().id()), XdrStoryData,
std::move(story_data), std::move(result_call));
};
class CreateStoryCall : public LedgerOperation<fidl::StringPtr, fuchsia::ledger::PageId> {
public:
CreateStoryCall(fuchsia::ledger::Ledger* const ledger, fuchsia::ledger::Page* const root_page,
- fidl::StringPtr story_name,
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info,
- fuchsia::modular::StoryOptions story_options, ResultCall result_call)
+ fidl::StringPtr story_name, fuchsia::modular::StoryOptions story_options,
+ ResultCall result_call)
: LedgerOperation("SessionStorage::CreateStoryCall", ledger, root_page,
std::move(result_call)),
story_name_(std::move(story_name)),
- extra_info_(std::move(extra_info)),
story_options_(std::move(story_options)) {}
private:
@@ -98,14 +96,12 @@
story_data_->set_story_name(story_name_.value_or(""));
story_data_->set_story_options(std::move(story_options_));
story_data_->set_story_page_id(std::move(story_page_id_));
- story_data_->mutable_story_info()->id = story_name_.value_or("");
- story_data_->mutable_story_info()->last_focus_time = 0;
- story_data_->mutable_story_info()->extra = std::move(extra_info_);
+ story_data_->mutable_story_info()->set_id(story_name_.value_or(""));
+ story_data_->mutable_story_info()->set_last_focus_time(0);
operation_queue_.Add(MakeWriteStoryDataCall(page(), std::move(story_data_), [flow] {}));
}
fidl::StringPtr story_name_;
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info_;
fuchsia::modular::StoryOptions story_options_;
fuchsia::ledger::PagePtr story_page_;
@@ -120,20 +116,18 @@
} // namespace
FuturePtr<fidl::StringPtr, fuchsia::ledger::PageId> SessionStorage::CreateStory(
- fidl::StringPtr story_name, fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info,
- fuchsia::modular::StoryOptions story_options) {
+ fidl::StringPtr story_name, fuchsia::modular::StoryOptions story_options) {
auto ret =
Future<fidl::StringPtr, fuchsia::ledger::PageId>::Create("SessionStorage.CreateStory.ret");
- operation_queue_.Add(std::make_unique<CreateStoryCall>(
- ledger_client_->ledger(), page(), std::move(story_name), std::move(extra_info),
- std::move(story_options), ret->Completer()));
+ operation_queue_.Add(
+ std::make_unique<CreateStoryCall>(ledger_client_->ledger(), page(), std::move(story_name),
+ std::move(story_options), ret->Completer()));
return ret;
}
FuturePtr<fidl::StringPtr, fuchsia::ledger::PageId> SessionStorage::CreateStory(
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info,
fuchsia::modular::StoryOptions story_options) {
- return CreateStory(nullptr /* story_name */, std::move(extra_info), std::move(story_options));
+ return CreateStory(/*story_name=*/nullptr, std::move(story_options));
}
namespace {
@@ -165,9 +159,9 @@
story_page_.NewRequest());
story_page_->Clear();
// Remove the story data in the session page.
- session_page_->Delete(to_array(StoryNameToStoryDataKey(story_data_.story_info().id)));
+ session_page_->Delete(to_array(StoryNameToStoryDataKey(story_data_.story_info().id())));
// Remove the story snapshot in the session page.
- session_page_->Delete(to_array(StoryNameToStorySnapshotKey(story_data_.story_info().id)));
+ session_page_->Delete(to_array(StoryNameToStorySnapshotKey(story_data_.story_info().id())));
}
fuchsia::ledger::Ledger* const ledger_; // not owned
@@ -230,10 +224,10 @@
FuturePtr<> SessionStorage::UpdateLastFocusedTimestamp(fidl::StringPtr story_name,
const int64_t ts) {
auto mutate = [ts](fuchsia::modular::internal::StoryData* const story_data) {
- if (story_data->story_info().last_focus_time == ts) {
+ if (story_data->story_info().last_focus_time() == ts) {
return false;
}
- story_data->mutable_story_info()->last_focus_time = ts;
+ story_data->mutable_story_info()->set_last_focus_time(ts);
return true;
};
@@ -345,18 +339,19 @@
FlowToken flow{this, &snapshot_};
page_snapshot_ = page_client_->NewSnapshot();
- page_snapshot_->Get(to_array(key_.value_or("")), [this,
- flow](fuchsia::ledger::PageSnapshot_Get_Result result) {
- if (result.is_response()) {
- snapshot_ = fidl::MakeOptional(std::move(result.response().buffer));
- return;
- }
- if (result.err() == fuchsia::ledger::Error::KEY_NOT_FOUND) {
- return;
- }
- // TODO(MI4-1425): Handle NEEDS_FETCH status if using lazy priority.
- FXL_LOG(ERROR) << trace_name() << " PageSnapshot.Get() " << fidl::ToUnderlying(result.err());
- });
+ page_snapshot_->Get(to_array(key_.value_or("")),
+ [this, flow](fuchsia::ledger::PageSnapshot_Get_Result result) {
+ if (result.is_response()) {
+ snapshot_ = fidl::MakeOptional(std::move(result.response().buffer));
+ return;
+ }
+ if (result.err() == fuchsia::ledger::Error::KEY_NOT_FOUND) {
+ return;
+ }
+ // TODO(MI4-1425): Handle NEEDS_FETCH status if using lazy priority.
+ FXL_LOG(ERROR) << trace_name() << " PageSnapshot.Get() "
+ << fidl::ToUnderlying(result.err());
+ });
}
// Input parameters.
diff --git a/peridot/bin/sessionmgr/storage/session_storage.h b/peridot/bin/sessionmgr/storage/session_storage.h
index 7750463..7468d04 100644
--- a/peridot/bin/sessionmgr/storage/session_storage.h
+++ b/peridot/bin/sessionmgr/storage/session_storage.h
@@ -57,13 +57,10 @@
}
// Creates a new story and returns a tuple of (story id, story ledger page
- // id) on completion. |story_name| and |extra_info| may be null.
+ // id) on completion. |story_name| may be null.
//
// If |story_name| is not provided, a UUID will be generated as the name.
//
- // If |extra_info| is set, populates StoryData.story_info.extra with the
- // entries given.
- //
// TODO(thatguy): Allowing for null story names is left in for backwards
// compatibility with existing code. The intention is that all clients
// outside the FW (through FIDL interfaces) use story names exclusively. It
@@ -71,12 +68,10 @@
// SessionStorage, or if they should be exposed to the story runtime
// architecture.
FuturePtr<fidl::StringPtr, fuchsia::ledger::PageId> CreateStory(
- fidl::StringPtr story_name, fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info,
- fuchsia::modular::StoryOptions story_options);
+ fidl::StringPtr story_name, fuchsia::modular::StoryOptions story_options);
// Same as above, but defaults |story_name| to nullptr.
FuturePtr<fidl::StringPtr, fuchsia::ledger::PageId> CreateStory(
- fidl::VectorPtr<fuchsia::modular::StoryInfoExtraEntry> extra_info,
fuchsia::modular::StoryOptions story_options);
// Deletes the |story_id| from the list of known stories and completes the
diff --git a/peridot/bin/sessionmgr/storage/session_storage_unittest.cc b/peridot/bin/sessionmgr/storage/session_storage_unittest.cc
index c51f24f..40729d0 100644
--- a/peridot/bin/sessionmgr/storage/session_storage_unittest.cc
+++ b/peridot/bin/sessionmgr/storage/session_storage_unittest.cc
@@ -28,8 +28,7 @@
// we're not testing CreateStory().
fidl::StringPtr CreateStory(SessionStorage* storage,
fuchsia::modular::StoryOptions story_options = {}) {
- auto future_story =
- storage->CreateStory(nullptr /* name */, nullptr /* extra */, std::move(story_options));
+ auto future_story = storage->CreateStory(nullptr /* name */, std::move(story_options));
bool done{};
fidl::StringPtr story_name;
future_story->Then([&](fidl::StringPtr name, fuchsia::ledger::PageId page_id) {
@@ -47,20 +46,9 @@
// correct.
auto storage = CreateStorage("page");
- std::vector<fuchsia::modular::StoryInfoExtraEntry> extra_entries;
- fuchsia::modular::StoryInfoExtraEntry entry;
- entry.key = "key1";
- entry.value = "value1";
- extra_entries.push_back(std::move(entry));
-
- entry.key = "key2";
- entry.value = "value2";
- extra_entries.push_back(std::move(entry));
-
fuchsia::modular::StoryOptions story_options;
story_options.kind_of_proto_story = true;
- auto future_story =
- storage->CreateStory("story_name", std::move(extra_entries), std::move(story_options));
+ auto future_story = storage->CreateStory("story_name", std::move(story_options));
bool done{};
fidl::StringPtr story_name;
fuchsia::ledger::PageId page_id;
@@ -80,15 +68,9 @@
EXPECT_EQ("story_name", data->story_name());
EXPECT_TRUE(data->story_options().kind_of_proto_story);
- EXPECT_EQ(story_name, data->story_info().id);
+ EXPECT_EQ(story_name, data->story_info().id());
ASSERT_TRUE(data->has_story_page_id());
EXPECT_TRUE(fidl::Equals(page_id, data->story_page_id()));
- EXPECT_TRUE(data->story_info().extra);
- EXPECT_EQ(2u, data->story_info().extra->size());
- EXPECT_EQ("key1", data->story_info().extra->at(0).key);
- EXPECT_EQ("value1", data->story_info().extra->at(0).value);
- EXPECT_EQ("key2", data->story_info().extra->at(1).key);
- EXPECT_EQ("value2", data->story_info().extra->at(1).value);
done = true;
@@ -125,8 +107,7 @@
// Pipeline all the calls such to show that we data consistency based on call
// order.
auto storage = CreateStorage("page");
- auto future_story =
- storage->CreateStory("story_name", nullptr /* extra_info */, {} /* options */);
+ auto future_story = storage->CreateStory("story_name", /*story_options=*/{});
// Immediately after creation is complete, delete it.
FuturePtr<> delete_done;
@@ -163,8 +144,8 @@
// * If we GetAllStoryData() we should see both of them.
auto storage = CreateStorage("page");
- auto future_story1 = storage->CreateStory("story1", nullptr /* extra_info */, {} /* options */);
- auto future_story2 = storage->CreateStory("story2", nullptr /* extra_info */, {} /* options*/);
+ auto future_story1 = storage->CreateStory("story1", /*story_options=*/{});
+ auto future_story2 = storage->CreateStory("story2", /*story_options=*/{});
fidl::StringPtr story1_name;
fuchsia::ledger::PageId story1_pageid;
@@ -236,8 +217,7 @@
// When we call DeleteStory, we expect the story's page to be completely
// emptied.
auto storage = CreateStorage("page");
- auto future_story =
- storage->CreateStory("story_name", nullptr /* extra_info */, {} /* options */);
+ auto future_story = storage->CreateStory("story_name", /*story_options=*/{});
bool done{false};
auto story_page_id = fuchsia::ledger::PageId::New();
@@ -285,7 +265,7 @@
auto future_data = storage->GetStoryData(story_name);
bool done{};
future_data->Then([&](fuchsia::modular::internal::StoryDataPtr data) {
- EXPECT_EQ(10, data->story_info().last_focus_time);
+ EXPECT_EQ(10, data->story_info().last_focus_time());
done = true;
});
RunLoopUntil([&] { return done; });
@@ -314,14 +294,14 @@
auto created_story_name = CreateStory(storage.get());
RunLoopUntil([&] { return updated; });
EXPECT_EQ(created_story_name, updated_story_name);
- EXPECT_EQ(created_story_name, updated_story_data.story_info().id);
+ EXPECT_EQ(created_story_name, updated_story_data.story_info().id());
// Update something and see a new notification.
updated = false;
storage->UpdateLastFocusedTimestamp(created_story_name, 42);
RunLoopUntil([&] { return updated; });
EXPECT_EQ(created_story_name, updated_story_name);
- EXPECT_EQ(42, updated_story_data.story_info().last_focus_time);
+ EXPECT_EQ(42, updated_story_data.story_info().last_focus_time());
// Update options and see a new notification.
updated = false;
@@ -330,7 +310,7 @@
storage->UpdateStoryOptions(created_story_name, std::move(story_options));
RunLoopUntil([&] { return updated; });
EXPECT_EQ(created_story_name, updated_story_name);
- EXPECT_EQ(created_story_name, updated_story_data.story_info().id);
+ EXPECT_EQ(created_story_name, updated_story_data.story_info().id());
EXPECT_TRUE(updated_story_data.story_options().kind_of_proto_story);
// Delete the story and expect to see a notification.
@@ -366,14 +346,14 @@
auto created_story_name = CreateStory(remote_storage.get());
RunLoopUntil([&] { return updated; });
EXPECT_EQ(created_story_name, updated_story_name);
- EXPECT_EQ(created_story_name, updated_story_data.story_info().id);
+ EXPECT_EQ(created_story_name, updated_story_data.story_info().id());
// Update something and see a new notification.
updated = false;
remote_storage->UpdateLastFocusedTimestamp(created_story_name, 42);
RunLoopUntil([&] { return updated; });
EXPECT_EQ(created_story_name, updated_story_name);
- EXPECT_EQ(42, updated_story_data.story_info().last_focus_time);
+ EXPECT_EQ(42, updated_story_data.story_info().last_focus_time());
// Update options and see a new notification.
updated = false;
@@ -382,7 +362,7 @@
remote_storage->UpdateStoryOptions(created_story_name, std::move(story_options));
RunLoopUntil([&] { return updated; });
EXPECT_EQ(created_story_name, updated_story_name);
- EXPECT_EQ(created_story_name, updated_story_data.story_info().id);
+ EXPECT_EQ(created_story_name, updated_story_data.story_info().id());
EXPECT_TRUE(updated_story_data.story_options().kind_of_proto_story);
// Delete the story and expect to see a notification.
diff --git a/peridot/bin/sessionmgr/storage/session_storage_xdr.cc b/peridot/bin/sessionmgr/storage/session_storage_xdr.cc
index 48e7f90..e071896 100644
--- a/peridot/bin/sessionmgr/storage/session_storage_xdr.cc
+++ b/peridot/bin/sessionmgr/storage/session_storage_xdr.cc
@@ -50,48 +50,27 @@
// Serialization and deserialization of fuchsia::modular::internal::StoryData
// and fuchsia::modular::StoryInfo to and from JSON. We have different versions
// for backwards compatibilty.
-//
+
+void XdrStoryInfo2_v0(XdrContext* const xdr, fuchsia::modular::StoryInfo2* const data) {
+ xdr->Field("id", data->mutable_id());
+ xdr->Field("last_focus_time", data->mutable_last_focus_time());
+}
+
// Version 0: Before FIDL2 conversion. ExtraInfo fields are stored as "key"
// and "value", page ids are stored as vector.
-void XdrStoryInfoExtraEntry_v0(XdrContext* const xdr,
- fuchsia::modular::StoryInfoExtraEntry* const data) {
- xdr->Field("key", &data->key);
- xdr->Field("value", &data->value);
-}
-
-void XdrStoryInfo_v0(XdrContext* const xdr, fuchsia::modular::StoryInfo* const data) {
- xdr->Field("last_focus_time", &data->last_focus_time);
- xdr->Field("url", &data->url);
- xdr->Field("id", &data->id);
- xdr->Field("extra", &data->extra, XdrStoryInfoExtraEntry_v0);
-}
-
void XdrStoryData_v0(XdrContext* const xdr, fuchsia::modular::internal::StoryData* const data) {
FXL_CHECK(xdr->op() == XdrOp::FROM_JSON) << "A back version is never used for writing.";
data->set_story_page_id(fuchsia::ledger::PageId());
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v0);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
xdr->Field("story_page_id", &data->mutable_story_page_id()->id);
}
// Version 1: During FIDL2 conversion. ExtraInfo fields are stored as "key"
// and "value", page ids are stored as base64 string.
-void XdrStoryInfoExtraEntry_v1(XdrContext* const xdr,
- fuchsia::modular::StoryInfoExtraEntry* const data) {
- xdr->Field("key", &data->key);
- xdr->Field("value", &data->value);
-}
-
-void XdrStoryInfo_v1(XdrContext* const xdr, fuchsia::modular::StoryInfo* const data) {
- xdr->Field("last_focus_time", &data->last_focus_time);
- xdr->Field("url", &data->url);
- xdr->Field("id", &data->id);
- xdr->Field("extra", &data->extra, XdrStoryInfoExtraEntry_v1);
-}
-
void XdrStoryData_v1(XdrContext* const xdr, fuchsia::modular::internal::StoryData* const data) {
static constexpr char kStoryPageId[] = "story_page_id";
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v1);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
switch (xdr->op()) {
case XdrOp::FROM_JSON: {
std::string page_id;
@@ -116,25 +95,12 @@
// Version 2: After FIDL2 conversion was complete. ExtraInfo fields are stored
// as @k and @v, page ids are stored as array wrapped in a struct.
-void XdrStoryInfoExtraEntry_v2(XdrContext* const xdr,
- fuchsia::modular::StoryInfoExtraEntry* const data) {
- xdr->Field("@k", &data->key);
- xdr->Field("@v", &data->value);
-}
-
-void XdrStoryInfo_v2(XdrContext* const xdr, fuchsia::modular::StoryInfo* const data) {
- xdr->Field("last_focus_time", &data->last_focus_time);
- xdr->Field("url", &data->url);
- xdr->Field("id", &data->id);
- xdr->Field("extra", &data->extra, XdrStoryInfoExtraEntry_v2);
-}
-
void XdrPageId_v2(XdrContext* const xdr, fuchsia::ledger::PageId* const data) {
xdr->Field("id", &data->id);
}
void XdrStoryData_v2(XdrContext* const xdr, fuchsia::modular::internal::StoryData* const data) {
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v2);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
xdr->Field("story_page_id", data->mutable_story_page_id(), XdrPageId_v2);
}
@@ -146,7 +112,7 @@
}
// NOTE(mesch): We reuse subsidiary filters of previous versions as long as we
// can. Only when they change too we create new versions of them.
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v2);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
xdr->Field("story_page_id", data->mutable_story_page_id(), XdrPageId_v2);
}
@@ -157,7 +123,7 @@
}
// NOTE(mesch): We reuse subsidiary filters of previous versions as long as we
// can. Only when they change too we create new versions of them.
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v2);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
xdr->Field("story_page_id", data->mutable_story_page_id(), XdrPageId_v2);
}
@@ -168,7 +134,7 @@
}
// NOTE(mesch): We reuse subsidiary filters of previous versions as long as we
// can. Only when they change too we create new versions of them.
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v2);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
xdr->Field("story_name", data->mutable_story_name());
xdr->Field("story_page_id", data->mutable_story_page_id(), XdrPageId_v2);
}
@@ -184,7 +150,7 @@
}
// NOTE(mesch): We reuse subsidiary filters of previous versions as long as we
// can. Only when they change too we create new versions of them.
- xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo_v2);
+ xdr->Field("story_info", data->mutable_story_info(), XdrStoryInfo2_v0);
xdr->Field("story_name", data->mutable_story_name());
xdr->Field("story_options", data->mutable_story_options(), XdrStoryOptions_v1);
xdr->Field("story_page_id", data->mutable_story_page_id(), XdrPageId_v2);
diff --git a/peridot/bin/sessionmgr/story_runner/story_controller_impl.cc b/peridot/bin/sessionmgr/story_runner/story_controller_impl.cc
index 20e54c02..8f18d87 100644
--- a/peridot/bin/sessionmgr/story_runner/story_controller_impl.cc
+++ b/peridot/bin/sessionmgr/story_runner/story_controller_impl.cc
@@ -1223,18 +1223,18 @@
// If this call enters a race with a StoryProvider.DeleteStory() call,
// resulting in |this| being destroyed, |callback| will be dropped.
operation_queue_.Add(std::make_unique<SyncCall>([this, callback = std::move(callback)] {
- auto story_info = story_provider_impl_->GetCachedStoryInfo(story_id_.value_or(""));
- FXL_CHECK(story_info);
- callback(std::move(*story_info), story_observer_->model().runtime_state());
+ auto story_info_2 = story_provider_impl_->GetCachedStoryInfo(story_id_.value_or(""));
+ FXL_CHECK(story_info_2);
+ auto story_info = modular::StoryProviderImpl::StoryInfo2ToStoryInfo(*story_info_2);
+ callback(std::move(story_info), story_observer_->model().runtime_state());
}));
}
void StoryControllerImpl::GetInfo2(GetInfo2Callback callback) {
operation_queue_.Add(std::make_unique<SyncCall>([this, callback = std::move(callback)] {
- auto story_info = story_provider_impl_->GetCachedStoryInfo(story_id_);
- FXL_CHECK(story_info);
- auto story_info_2 = story_provider_impl_->StoryInfoToStoryInfo2(*story_info);
- callback(std::move(story_info_2), story_observer_->model().runtime_state());
+ auto story_info_2 = story_provider_impl_->GetCachedStoryInfo(story_id_);
+ FXL_CHECK(story_info_2);
+ callback(std::move(*story_info_2), story_observer_->model().runtime_state());
}));
}
diff --git a/peridot/bin/sessionmgr/story_runner/story_provider_impl.cc b/peridot/bin/sessionmgr/story_runner/story_provider_impl.cc
index 95eabe8..a5af91bd 100644
--- a/peridot/bin/sessionmgr/story_runner/story_provider_impl.cc
+++ b/peridot/bin/sessionmgr/story_runner/story_provider_impl.cc
@@ -359,10 +359,10 @@
for (const auto& item : story_runtime_containers_) {
const auto& container = item.second;
FXL_CHECK(container.current_data->has_story_info());
- watcher_ptr->OnChange(CloneStruct(container.current_data->story_info()),
+ watcher_ptr->OnChange(StoryInfo2ToStoryInfo(container.current_data->story_info()),
container.model_observer->model().runtime_state(),
container.model_observer->model().visibility_state());
- watcher_ptr->OnChange2(StoryInfoToStoryInfo2(container.current_data->story_info()),
+ watcher_ptr->OnChange2(CloneStruct(container.current_data->story_info()),
container.model_observer->model().runtime_state(),
container.model_observer->model().visibility_state());
}
@@ -455,7 +455,7 @@
user_environment_->GetLauncher(), CloneStruct(story_shell_config_));
}
-fuchsia::modular::StoryInfoPtr StoryProviderImpl::GetCachedStoryInfo(std::string story_id) {
+fuchsia::modular::StoryInfo2Ptr StoryProviderImpl::GetCachedStoryInfo(std::string story_id) {
auto it = story_runtime_containers_.find(story_id);
if (it == story_runtime_containers_.end()) {
return nullptr;
@@ -477,7 +477,7 @@
if (!story_data->has_story_info()) {
return nullptr;
}
- return fidl::MakeOptional(std::move(*story_data->mutable_story_info()));
+ return fidl::MakeOptional(StoryInfo2ToStoryInfo(story_data->story_info()));
});
operation_queue_.Add(
WrapFutureAsOperation("StoryProviderImpl::GetStoryInfo", on_run, done, std::move(callback)));
@@ -496,8 +496,7 @@
if (!story_data->has_story_info()) {
return fuchsia::modular::StoryInfo2{};
}
-
- return StoryInfoToStoryInfo2(story_data->story_info());
+ return std::move(*story_data->mutable_story_info());
});
operation_queue_.Add(
WrapFutureAsOperation("StoryProviderImpl::GetStoryInfo2", on_run, done, std::move(callback)));
@@ -577,7 +576,7 @@
if (!story_data.has_story_info()) {
continue;
}
- result.push_back(std::move(*story_data.mutable_story_info()));
+ result.push_back(StoryInfo2ToStoryInfo(story_data.story_info()));
}
}
@@ -608,7 +607,7 @@
if (!story_data.has_story_info()) {
continue;
}
- result.push_back(StoryInfoToStoryInfo2(story_data.story_info()));
+ result.push_back(std::move(*story_data.mutable_story_info()));
}
}
@@ -634,7 +633,7 @@
if (!story_data.has_story_info()) {
continue;
}
- result.push_back(std::move(*story_data.mutable_story_info()));
+ result.push_back(StoryInfo2ToStoryInfo(story_data.story_info()));
}
}
return result;
@@ -655,7 +654,7 @@
if (!story_data.has_story_info()) {
continue;
}
- result.push_back(StoryInfoToStoryInfo2(story_data.story_info()));
+ result.push_back(std::move(*story_data.mutable_story_info()));
}
}
return result;
@@ -674,7 +673,7 @@
fuchsia::modular::StoryState runtime_state = fuchsia::modular::StoryState::STOPPED;
fuchsia::modular::StoryVisibilityState visibility_state =
fuchsia::modular::StoryVisibilityState::DEFAULT;
- auto i = story_runtime_containers_.find(story_data.story_info().id);
+ auto i = story_runtime_containers_.find(story_data.story_info().id());
if (i != story_runtime_containers_.end()) {
runtime_state = i->second.model_observer->model().runtime_state();
visibility_state = i->second.model_observer->model().visibility_state();
@@ -739,9 +738,9 @@
if (!story_data->has_story_info()) {
continue;
}
- (*i)->OnChange(CloneStruct(story_data->story_info()), story_state, story_visibility_state);
- (*i)->OnChange2(StoryInfoToStoryInfo2(story_data->story_info()), story_state,
- story_visibility_state);
+ (*i)->OnChange(StoryInfo2ToStoryInfo(story_data->story_info()), story_state,
+ story_visibility_state);
+ (*i)->OnChange2(CloneStruct(story_data->story_info()), story_state, story_visibility_state);
}
}
@@ -834,14 +833,14 @@
loader_request.TakeChannel());
}
-fuchsia::modular::StoryInfo2 StoryProviderImpl::StoryInfoToStoryInfo2(
- const fuchsia::modular::StoryInfo& story_info) {
- fuchsia::modular::StoryInfo2 story_info_2;
+fuchsia::modular::StoryInfo StoryProviderImpl::StoryInfo2ToStoryInfo(
+ const fuchsia::modular::StoryInfo2& story_info_2) {
+ fuchsia::modular::StoryInfo story_info;
- story_info_2.set_id(story_info.id);
- story_info_2.set_last_focus_time(story_info.last_focus_time);
+ story_info.id = story_info_2.id();
+ story_info.last_focus_time = story_info_2.last_focus_time();
- return story_info_2;
+ return story_info;
}
} // namespace modular
diff --git a/peridot/bin/sessionmgr/story_runner/story_provider_impl.h b/peridot/bin/sessionmgr/story_runner/story_provider_impl.h
index aff1f33..9401edb 100644
--- a/peridot/bin/sessionmgr/story_runner/story_provider_impl.h
+++ b/peridot/bin/sessionmgr/story_runner/story_provider_impl.h
@@ -115,7 +115,7 @@
// Called by StoryControllerImpl.
//
// Returns nullptr if the StoryInfo for |story_id| is not cached.
- fuchsia::modular::StoryInfoPtr GetCachedStoryInfo(std::string story_id);
+ fuchsia::modular::StoryInfo2Ptr GetCachedStoryInfo(std::string story_id);
// |fuchsia::modular::StoryProvider|.
void GetStoryInfo(std::string story_id, GetStoryInfoCallback callback) override;
@@ -178,9 +178,9 @@
const std::string& story_id,
fidl::InterfaceRequest<fuchsia::modular::EntityProvider> entity_provider_request);
- // Converts a StoryInfo to StoryInfo2.
- static fuchsia::modular::StoryInfo2 StoryInfoToStoryInfo2(
- const fuchsia::modular::StoryInfo& story_info);
+ // Converts a StoryInfo2 to StoryInfo.
+ static fuchsia::modular::StoryInfo StoryInfo2ToStoryInfo(
+ const fuchsia::modular::StoryInfo2& story_info_2);
private:
// |fuchsia::modular::StoryProvider|
diff --git a/sdk/fidl/fuchsia.modular.internal/story_data.fidl b/sdk/fidl/fuchsia.modular.internal/story_data.fidl
index d2abd7c..f48615a 100644
--- a/sdk/fidl/fuchsia.modular.internal/story_data.fidl
+++ b/sdk/fidl/fuchsia.modular.internal/story_data.fidl
@@ -11,7 +11,7 @@
/// data necessary to run a story: see story_model.fidl for that.
table StoryData {
/// Metadata available to the SessionShell.
- 1: fuchsia.modular.StoryInfo story_info;
+ 1: fuchsia.modular.StoryInfo2 story_info;
/// A client-supplied name for this story.
2: string story_name;
diff --git a/sdk/fidl/fuchsia.modular/fuchsia.modular.api b/sdk/fidl/fuchsia.modular/fuchsia.modular.api
index 47a2ba7..ad287a8 100644
--- a/sdk/fidl/fuchsia.modular/fuchsia.modular.api
+++ b/sdk/fidl/fuchsia.modular/fuchsia.modular.api
@@ -35,7 +35,7 @@
"fidl/fuchsia.modular/story/create_link.fidl": "f408177279214b9b2bb6308909f62899",
"fidl/fuchsia.modular/story/create_module_parameter_map.fidl": "78f0143e867d14b58e80b194962c4717",
"fidl/fuchsia.modular/story/link.fidl": "585299f92ef6ab346645d538c1056a52",
- "fidl/fuchsia.modular/story/puppet_master.fidl": "5c833c9f56823c2c1b09d627319ac9a2",
+ "fidl/fuchsia.modular/story/puppet_master.fidl": "a4b5f4e852baf24c5f5e6efa1af90605",
"fidl/fuchsia.modular/story/story_command.fidl": "b400b93da872e5e73061a642bade926d",
"fidl/fuchsia.modular/story/story_controller.fidl": "3dd0f59993ff88e32f91df82efad0f77",
"fidl/fuchsia.modular/story/story_info.fidl": "6fcb8feb403fb389cbe5a737657649af",
diff --git a/sdk/fidl/fuchsia.modular/story/puppet_master.fidl b/sdk/fidl/fuchsia.modular/story/puppet_master.fidl
index 72717c3..3ca1db4 100644
--- a/sdk/fidl/fuchsia.modular/story/puppet_master.fidl
+++ b/sdk/fidl/fuchsia.modular/story/puppet_master.fidl
@@ -101,6 +101,9 @@
// Any subsequent calls (either on the same channel or on a new
// StoryPuppetMaster for an existing story) are ignored.
// See story_info.fidl for details.
+ //
+ // DEPRECATED. This method is a no-op and will never return an error.
+ [Transitional = "Deprecated, no-op"]
SetStoryInfoExtra(vector<StoryInfoExtraEntry> story_info_extra)
-> () error ConfigureStoryError;
};
