[intent] Remove links from intents
This removes the link-based intent parameters.
MF-6 #done
TESTED=Existing tests.
Change-Id: Icc56d62dbc4fa2938fafa3afb768bb336761d02b
diff --git a/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc b/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc
index 4327806..aa187c4 100644
--- a/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc
+++ b/bin/sessionmgr/puppet_master/command_runners/add_mod_command_runner_unittest.cc
@@ -116,16 +116,6 @@
}
break;
}
- case fuchsia::modular::IntentParameterData::Tag::kLinkName:
- if (old_param.link_name() != new_param.link_name()) {
- return false;
- }
- break;
- case fuchsia::modular::IntentParameterData::Tag::kLinkPath:
- if (old_param.link_path() != new_param.link_path()) {
- return false;
- }
- break;
case fuchsia::modular::IntentParameterData::Tag::Invalid:
break;
}
@@ -202,24 +192,6 @@
intent->parameters.push_back(std::move(parameter));
}
- void AddLinkNameParameter(fuchsia::modular::Intent* intent,
- const std::string& name,
- const std::string& link_name) {
- fuchsia::modular::IntentParameter parameter;
- parameter.name = name;
- parameter.data.set_link_name(link_name);
- intent->parameters.push_back(std::move(parameter));
- }
-
- void AddLinkPathParameter(fuchsia::modular::Intent* intent,
- const std::string& name,
- const std::string& link_path) {
- fuchsia::modular::IntentParameter parameter;
- parameter.name = name;
- parameter.data.set_link_path(MakeLinkPath(link_path));
- intent->parameters.push_back(std::move(parameter));
- }
-
void AddInvalidParameter(fuchsia::modular::Intent* intent,
const std::string& name) {
// This parameter has no data union field set, hence it's invalid.
@@ -390,9 +362,6 @@
AddJsonParameter(&intent, "param_json", R"({"@type": "foo"})");
AddEntityRefParameter(&intent, "param_ref", reference);
AddEntityTypeParameter(&intent, "param_type", {"entity_type2"});
- SetLinkValue(story_storage_.get(), "link_path1", R"({"@type": "bar"})");
- AddLinkPathParameter(&intent, "param_linkpath", "link_path1");
- AddLinkNameParameter(&intent, "param_linkname", "parent_link_name");
auto command = MakeAddModCommand("mod", "parent_mod", 0.5, intent);
auto manifest = fuchsia::modular::ModuleManifest::New();
@@ -409,7 +378,7 @@
[&](const fuchsia::modular::FindModulesQuery& query) {
EXPECT_EQ("intent_action", query.action);
auto& constraints = query.parameter_constraints;
- EXPECT_EQ(5u, constraints.size());
+ EXPECT_EQ(3u, constraints.size());
for (auto& constraint : constraints) {
EXPECT_EQ(1u, constraint.param_types.size());
}
@@ -421,10 +390,6 @@
EXPECT_EQ("entity_type1", constraints.at(1).param_types.at(0));
EXPECT_EQ("param_type", constraints.at(2).param_name);
EXPECT_EQ("entity_type2", constraints.at(2).param_types.at(0));
- EXPECT_EQ("param_linkpath", constraints.at(3).param_name);
- EXPECT_EQ("bar", constraints.at(3).param_types.at(0));
- EXPECT_EQ("param_linkname", constraints.at(4).param_name);
- EXPECT_EQ("baz", constraints.at(4).param_types.at(0));
});
bool done{};
@@ -454,7 +419,7 @@
EXPECT_EQ(0.5, module_data->surface_relation->emphasis);
EXPECT_TRUE(AreIntentsEqual(intent, *module_data->intent));
EXPECT_EQ(*manifest, *module_data->module_manifest);
- EXPECT_EQ(5u, module_data->parameter_map.entries.size());
+ EXPECT_EQ(3u, module_data->parameter_map.entries.size());
// Verify parameters
auto& link_path1 = module_data->parameter_map.entries.at(0).link_path;
@@ -476,21 +441,6 @@
EXPECT_EQ(full_path, link_path3.module_path);
EXPECT_EQ("param_type", link_path3.link_name);
EXPECT_EQ("null", GetLinkValue(story_storage_.get(), link_path3));
-
- auto& link_path4 = module_data->parameter_map.entries.at(3).link_path;
- EXPECT_EQ("param_linkpath", module_data->parameter_map.entries.at(3).name);
- EXPECT_TRUE(link_path4.module_path.empty());
- EXPECT_EQ("link_path1", link_path4.link_name);
- EXPECT_EQ(R"({"@type": "bar"})",
- GetLinkValue(story_storage_.get(), link_path4));
-
- auto& link_path5 = module_data->parameter_map.entries.at(4).link_path;
- EXPECT_EQ("param_linkname", module_data->parameter_map.entries.at(4).name);
- EXPECT_EQ(1u, link_path5.module_path.size());
- EXPECT_EQ("parent_mod", link_path5.module_path.at(0));
- EXPECT_EQ("parent_link_name", link_path5.link_name);
- EXPECT_EQ(R"({"@type": "baz"})",
- GetLinkValue(story_storage_.get(), link_path5));
}
TEST_F(AddModCommandRunnerTest, ExecuteNoModulesFound) {
@@ -597,63 +547,6 @@
RunLoopUntil([&] { return done; });
}
-TEST_F(AddModCommandRunnerTest, ExecuteNoLinkPathForLinkName) {
- // Set up command
- auto intent = CreateEmptyIntent("intent_action");
- AddLinkNameParameter(&intent, "invalid_param", "parent_linkname");
- auto command = MakeAddModCommand("mod", "parent_mod", 0.5, intent);
-
- // Set up fake module resolver.
- auto manifest = fuchsia::modular::ModuleManifest::New();
- manifest->binary = "mod_url";
- manifest->intent_filters.push_back(MakeIntentFilter("intent_action", {}));
- fuchsia::modular::FindModulesResult result;
- result.module_id = "mod_url";
- fidl::Clone(manifest, &result.manifest);
- fake_module_resolver_->AddFindModulesResult(std::move(result));
-
- // Run the command and assert results.
- bool done{};
- runner_->Execute(story_id_, story_storage_.get(), std::move(command),
- [&done](fuchsia::modular::ExecuteResult result) {
- EXPECT_EQ(fuchsia::modular::ExecuteStatus::INVALID_COMMAND,
- result.status);
- EXPECT_EQ(
- "No link path found for parameter with name "
- "invalid_param",
- result.error_message);
- done = true;
- });
- RunLoopUntil([&done] { return done; });
-}
-
-// An empty json link should evaluate to a type of "com.google.fuchsia.unknown"
-TEST_F(AddModCommandRunnerTest, ExecuteEmptyJsonLinkPathParam) {
- // Set up command
- auto intent = CreateEmptyIntent("intent_action");
- AddLinkPathParameter(&intent, "invalid_param", "invalid_linkpath");
- auto command = MakeAddModCommand("mod", "parent_mod", 0.5, intent);
-
- // Set up fake module resolver.
- auto manifest = fuchsia::modular::ModuleManifest::New();
- manifest->binary = "mod_url";
- manifest->intent_filters.push_back(MakeIntentFilter("intent_action", {}));
- fuchsia::modular::FindModulesResult result;
- result.module_id = "mod_url";
- fidl::Clone(manifest, &result.manifest);
- fake_module_resolver_->AddFindModulesResult(std::move(result));
-
- // Run the command and assert results.
- bool done{};
- runner_->Execute(story_id_, story_storage_.get(), std::move(command),
- [&](fuchsia::modular::ExecuteResult result) {
- EXPECT_EQ(fuchsia::modular::ExecuteStatus::OK,
- result.status);
- done = true;
- });
- RunLoopUntil([&] { return done; });
-}
-
TEST_F(AddModCommandRunnerTest, ExecuteInvalidJsonParamResolution) {
// Set up command
auto intent = CreateEmptyIntent("intent_action");
diff --git a/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc b/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc
index f7205b8..41eca48 100644
--- a/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc
+++ b/bin/sessionmgr/puppet_master/command_runners/operation_calls/add_mod_call.cc
@@ -57,7 +57,7 @@
// forgivingly execute the handler anyway.
if (!intent_.action.is_null()) {
AddFindModulesOperation(
- &operation_queue_, story_storage_, module_resolver_, entity_resolver_,
+ &operation_queue_, module_resolver_, entity_resolver_,
CloneOptional(intent_), surface_parent_mod_name_,
[this, flow](fuchsia::modular::ExecuteResult result,
fuchsia::modular::FindModulesResponse response) {
@@ -170,45 +170,6 @@
entry.value.set_create_link(std::move(create_link));
break;
}
- case fuchsia::modular::IntentParameterData::Tag::kLinkName: {
- auto did_get_lp = Future<fuchsia::modular::LinkPathPtr>::Create(
- "AddModCommandRunner::AddModCall::did_get_link");
- // TODO(miguelfrde): get rid of using surface_parent_mod_name this
- // way. Maybe INVALID status should be returned here since using
- // this parameter in a StoryCommand doesn't make much sense.
- AddGetLinkPathForParameterNameOperation(
- &operations_, story_storage_, surface_parent_mod_name_,
- param.data.link_name(), did_get_lp->Completer());
-
- did_get_entries.emplace_back(
- did_get_lp->Map([this, param_name = param.name,
- done](fuchsia::modular::LinkPathPtr link_path) {
- if (!GetWeakPtr()) {
- return fuchsia::modular::CreateModuleParameterMapEntry{};
- }
- if (!link_path) {
- // OH NO! bail on this entire function's flow.
- out_result_.status =
- fuchsia::modular::ExecuteStatus::INTERNAL_ERROR;
- out_result_.error_message = fxl::StringPrintf(
- "Link path does not exist for parameter name = %s",
- param_name.get().c_str());
- done();
- return fuchsia::modular::CreateModuleParameterMapEntry{};
- }
- fuchsia::modular::CreateModuleParameterMapEntry entry;
- entry.key = param_name;
- entry.value.set_link_path(std::move(*link_path));
- return entry;
- }));
- continue;
- }
- case fuchsia::modular::IntentParameterData::Tag::kLinkPath: {
- fuchsia::modular::LinkPath lp;
- param.data.link_path().Clone(&lp);
- entry.value.set_link_path(std::move(lp));
- break;
- }
case fuchsia::modular::IntentParameterData::Tag::Invalid: {
out_result_.status = fuchsia::modular::ExecuteStatus::INVALID_COMMAND;
out_result_.error_message =
diff --git a/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.cc b/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.cc
index 08eeb5e..4bcb1e3 100644
--- a/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.cc
+++ b/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.cc
@@ -10,7 +10,6 @@
#include <lib/fxl/logging.h>
#include <lib/fxl/type_converter.h>
-#include "peridot/bin/sessionmgr/puppet_master/command_runners/operation_calls/get_link_path_for_parameter_name_call.h"
#include "peridot/bin/sessionmgr/puppet_master/command_runners/operation_calls/get_types_from_entity_call.h"
#include "peridot/lib/fidl/clone.h"
@@ -30,14 +29,12 @@
: public Operation<fuchsia::modular::ExecuteResult,
fuchsia::modular::FindModulesResponse> {
public:
- FindModulesCall(StoryStorage* const story_storage,
- fuchsia::modular::ModuleResolver* const module_resolver,
+ FindModulesCall(fuchsia::modular::ModuleResolver* const module_resolver,
fuchsia::modular::EntityResolver* const entity_resolver,
fuchsia::modular::IntentPtr intent,
std::vector<std::string> requesting_module_path,
ResultCall result_call)
: Operation("FindModulesCall", std::move(result_call)),
- story_storage_(story_storage),
module_resolver_(module_resolver),
entity_resolver_(entity_resolver),
intent_(std::move(intent)),
@@ -136,34 +133,6 @@
}
break;
}
- case fuchsia::modular::IntentParameterData::Tag::kLinkName: {
- auto did_get_lp = Future<fuchsia::modular::LinkPathPtr>::Create(
- "AddModCommandRunner::GetTypesFromIntentParameter.did_get_lp");
- AddGetLinkPathForParameterNameOperation(
- &operations_, story_storage_, requesting_module_path_,
- input.link_name(), did_get_lp->Completer());
- did_get_lp->Then([this, fut,
- param_name](fuchsia::modular::LinkPathPtr lp) {
- if (!lp) {
- std::ostringstream stream;
- stream << "No link path found for parameter with name "
- << param_name;
- result_.error_message = stream.str();
- result_.status = fuchsia::modular::ExecuteStatus::INVALID_COMMAND;
- fut->Complete({});
- } else {
- // If the call below has some error it will be set in result_.
- GetTypesFromLink(std::move(lp), fut->Completer(), param_name);
- }
- });
- break;
- }
- case fuchsia::modular::IntentParameterData::Tag::kLinkPath: {
- LinkPathPtr lp = CloneOptional(input.link_path());
- // If the call below has some error it will be set in result_.
- GetTypesFromLink(std::move(lp), fut->Completer(), param_name);
- break;
- }
case fuchsia::modular::IntentParameterData::Tag::Invalid: {
std::ostringstream stream;
stream << "Invalid data for parameter with name: " << param_name;
@@ -185,36 +154,6 @@
return std::nullopt;
}
- void GetTypesFromLink(fuchsia::modular::LinkPathPtr link_path,
- std::function<void(std::vector<std::string>)> done,
- const fidl::StringPtr& param_name) {
- story_storage_->GetLinkValue(*link_path)
- ->Then([this, done = std::move(done), param_name](
- StoryStorage::Status status, fidl::StringPtr v) {
- if (status != StoryStorage::Status::OK) {
- std::ostringstream stream;
- stream << "StoryStorage failed with status: " << (uint32_t)status
- << " for parameter with name " << param_name;
- result_.error_message = stream.str();
- result_.status = fuchsia::modular::ExecuteStatus::INTERNAL_ERROR;
- done({});
- return;
- }
- if (auto result = GetTypesFromJson(v)) {
- FXL_LOG(INFO) << "type=" << result.value()[0];
- done(*result);
- return;
- }
- std::ostringstream stream;
- stream << "Mal-formed JSON read from link for parameter: "
- << param_name;
- result_.error_message = stream.str();
- result_.status = fuchsia::modular::ExecuteStatus::INTERNAL_ERROR;
- done({});
- });
- }
-
- StoryStorage* const story_storage_; // Not owned.
fuchsia::modular::ModuleResolver* const module_resolver_; // Not Owned
fuchsia::modular::EntityResolver* const entity_resolver_; // Not owned.
const fuchsia::modular::IntentPtr intent_;
@@ -223,7 +162,6 @@
fuchsia::modular::FindModulesQuery resolver_query_;
std::vector<FuturePtr<fuchsia::modular::FindModulesParameterConstraint>>
constraint_futs_;
- fuchsia::modular::LinkPtr link_; // in case we need itf for
fuchsia::modular::ExecuteResult result_;
fuchsia::modular::FindModulesResponse response_;
OperationCollection operations_;
@@ -234,7 +172,7 @@
} // namespace
void AddFindModulesOperation(
- OperationContainer* operation_container, StoryStorage* const story_storage,
+ OperationContainer* operation_container,
fuchsia::modular::ModuleResolver* const module_resolver,
fuchsia::modular::EntityResolver* const entity_resolver,
fuchsia::modular::IntentPtr intent,
@@ -243,7 +181,7 @@
fuchsia::modular::FindModulesResponse)>
result_call) {
operation_container->Add(new FindModulesCall(
- story_storage, module_resolver, entity_resolver, std::move(intent),
+ module_resolver, entity_resolver, std::move(intent),
std::move(requesting_module_path), std::move(result_call)));
}
diff --git a/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.h b/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.h
index 97e6a67..dcd34af 100644
--- a/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.h
+++ b/bin/sessionmgr/puppet_master/command_runners/operation_calls/find_modules_call.h
@@ -7,12 +7,11 @@
#include <fuchsia/modular/cpp/fidl.h>
#include <lib/async/cpp/operation.h>
-#include "peridot/bin/sessionmgr/storage/story_storage.h"
namespace modular {
void AddFindModulesOperation(
- OperationContainer* operation_container, StoryStorage* story_storage,
+ OperationContainer* operation_container,
fuchsia::modular::ModuleResolver* module_resolver,
fuchsia::modular::EntityResolver* entity_resolver,
fuchsia::modular::IntentPtr intent,
diff --git a/bin/sessionmgr/storage/story_storage_xdr.cc b/bin/sessionmgr/storage/story_storage_xdr.cc
index 22fbde4..238cb99 100644
--- a/bin/sessionmgr/storage/story_storage_xdr.cc
+++ b/bin/sessionmgr/storage/story_storage_xdr.cc
@@ -45,8 +45,6 @@
static constexpr char kEntityReference[] = "entity_reference";
static constexpr char kJson[] = "json";
static constexpr char kEntityType[] = "entity_type";
- static constexpr char kLinkName[] = "link_name";
- static constexpr char kLinkPath[] = "link_path";
switch (xdr->op()) {
case XdrOp::FROM_JSON: {
@@ -67,14 +65,6 @@
::std::vector<::std::string> value;
xdr->Field(kEntityType, &value);
data->set_entity_type(std::move(value));
- } else if (tag == kLinkName) {
- fidl::StringPtr value;
- xdr->Field(kLinkName, &value);
- data->set_link_name(std::move(value));
- } else if (tag == kLinkPath) {
- fuchsia::modular::LinkPath value;
- xdr->Field(kLinkPath, &value, XdrLinkPath);
- data->set_link_path(std::move(value));
} else {
FXL_LOG(ERROR) << "XdrIntentParameterData FROM_JSON unknown tag: "
<< tag;
@@ -106,17 +96,6 @@
xdr->Field(kEntityType, &value);
break;
}
- case fuchsia::modular::IntentParameterData::Tag::kLinkName: {
- tag = kLinkName;
- fidl::StringPtr value = data->link_name();
- xdr->Field(kLinkName, &value);
- break;
- }
- case fuchsia::modular::IntentParameterData::Tag::kLinkPath: {
- tag = kLinkPath;
- xdr->Field(kLinkPath, &data->link_path(), XdrLinkPath);
- break;
- }
case fuchsia::modular::IntentParameterData::Tag::Invalid:
FXL_LOG(ERROR) << "XdrIntentParameterData TO_JSON unknown tag: "
<< static_cast<int>(data->Which());
diff --git a/bin/sessionmgr/story_runner/story_controller_impl.cc b/bin/sessionmgr/story_runner/story_controller_impl.cc
index 5e76939..4a196fc 100644
--- a/bin/sessionmgr/story_runner/story_controller_impl.cc
+++ b/bin/sessionmgr/story_runner/story_controller_impl.cc
@@ -257,18 +257,6 @@
fuchsia::modular::Intent intent;
module_data_.intent->Clone(&intent);
- for (auto& parameter : *intent.parameters) {
- // When an Intent is created the creator specifies a link either by
- // absolute path, or by name in the creator's namespace.
- //
- // The link_path and link_name both get converted to the handler's
- // namespace before the intent is sent to the handler. The link name in
- // the handler's namespace is always the same as the parameter name.
- if (parameter.data.is_link_path() || parameter.data.is_link_name()) {
- parameter.data.set_link_name(parameter.name);
- }
- }
-
intent_handler->HandleIntent(std::move(intent));
}
diff --git a/public/fidl/fuchsia.modular/intent/intent.fidl b/public/fidl/fuchsia.modular/intent/intent.fidl
index d2b07c9..09fb8ef 100644
--- a/public/fidl/fuchsia.modular/intent/intent.fidl
+++ b/public/fidl/fuchsia.modular/intent/intent.fidl
@@ -57,15 +57,4 @@
// Only one entry in |entity_type| must match the constraint specified by
// the Module for the constraint to be considered satisfied.
vector<string> entity_type;
-
- // Set this to use an existing Link accessible from your Module as the noun's
- // value.
- // TODO(thatguy): This is allowed to be null for backwards compatibility with
- // Mods that use the 'default link' (= a link with null name). Once no mods
- // use the default link any more, this can be made required again.
- // MI4-736
- string? link_name;
-
- // Set to re-use an existing link within a story.
- LinkPath link_path;
};
