ProjectContextFactory: TakeSingleProjectContext
We replace ProjectContextFactory::NewSingleProjectContext() with
ProjectContextFactory::TakeSingleProjectContext(). With this
new method it is not necessary to keep the ProjectContextFactory
alive because the returned ProjectContext owns the underlying
data.
CB-220
This is part of a series of CLs whose goal is to clean up the client-side
infrastrcutrue around accessing the Cobalt Registry.
Change-Id: Icdc0f6dfe952ced40a51f7a72dee9eede1aefd48
diff --git a/logger/project_context_factory.cc b/logger/project_context_factory.cc
index f36d021..0e0d4cca 100644
--- a/logger/project_context_factory.cc
+++ b/logger/project_context_factory.cc
@@ -68,14 +68,17 @@
release_stage);
}
-std::unique_ptr<ProjectContext> ProjectContextFactory::NewSingleProjectContext(
+std::unique_ptr<ProjectContext> ProjectContextFactory::TakeSingleProjectContext(
ReleaseStage release_stage) {
if (!is_single_project()) {
return nullptr;
}
- return NewProjectContext(project_configs_->single_customer_name(),
- project_configs_->single_project_name(),
- release_stage);
+ auto project_context = std::make_unique<ProjectContext>(
+ project_configs_->single_customer_id(),
+ project_configs_->single_customer_name(),
+ project_configs_->TakeSingleProjectConfig(), release_stage);
+ project_configs_.reset();
+ return project_context;
}
std::unique_ptr<encoder::ProjectContext>
diff --git a/logger/project_context_factory.h b/logger/project_context_factory.h
index 2e93d7e..343aabe 100644
--- a/logger/project_context_factory.h
+++ b/logger/project_context_factory.h
@@ -41,12 +41,13 @@
// ProjectContext.
//
// Depending on which state the factory's CobaltRegistry is in, invoke one
-// of the New*() methods to create a new ProjectContext.
+// of the New*() or Take*() methods to retrieve a new ProjectContext().
//
-// Important: The ProjectContextFactory continues to own its CobaltRegistry.
-// The returned ProjectContext maintains a pointer to the CobaltRegistry
-// owned by the ProjectContextFactory. Thus the ProjectContextFactory must
-// not be destructed until after the ProjectContext is no longer being used.
+// Important: Pay attention to the notes on each method regarding the
+// requirement that this ProjectContextFactory remain alive. In some cases,
+// the returned ProjectContext contains a poiner into the CobaltRegistry
+// owned by this ProjectContextFactory and so the factory must not
+// be destructed until after the ProjectContext is no longer being used.
class ProjectContextFactory {
public:
// Constructs a ProjectContextFactory whose CobaltRegistry is obtained
@@ -80,20 +81,26 @@
// contains that project. The ProjectContext will be marked as being for a
// client at the given |release_stage|. Returns nullptr otherwise.
//
- // This ProjectContextFactory must remain alive as long as the returned
- // ProjectContext is being used.
+ // Important: The returned ProjectContext contains a pointer into this
+ // factory's CobaltRegistry. This ProjectContextFactory must remain alive as
+ // long as the returned ProjectContext is being used.
std::unique_ptr<ProjectContext> NewProjectContext(
std::string customer_name, std::string project_name,
ReleaseStage release_stage = GA);
- // If is_single_project() is true, returns a ProjectContext for the unique
- // Cobalt 1.0 project contained in the factory's CobaltRegistry. The
- // ProjectContext will be marked as being for a client at the given
- // |release_stage|. Returns nullptr otherwise.
+ // If is_single_project() is true, then this returns a
+ // ProjectContext for the unique Cobalt 1.0 project contained in the factory's
+ // CobaltRegistry and removes the corresponding data from the registry,
+ // leaving this ProjectContextFactory invalid. The returned ProjectContext
+ // will be marked as being for a client at the given |release_stage|.
+ // Returns nullptr otherwise.
//
- // This ProjectContextFactory must remain alive as long as the returned
- // ProjectContext is being used.
- std::unique_ptr<ProjectContext> NewSingleProjectContext(
+ // Note: The returned ProjectContext does *not* contain a pointer into
+ // this factory's CobaltRegistry because the appropriate data is removed
+ // from the CobaltRegistry and is now owned by the ProjectContext. If nullptr
+ // is not returned then this ProjectContextFactory becomes invalid and
+ // shoud be discarded.
+ std::unique_ptr<ProjectContext> TakeSingleProjectContext(
ReleaseStage release_stage = GA);
// Returns a ProjectContext for the Cobalt 0.1 project with the given
@@ -103,8 +110,9 @@
// CobaltRegistry for the specified project so the returned ProjectContext
// may be non-null but still empty.
//
- // This ProjectContextFactory must remain alive as long as the returned
- // ProjectContext is being used.
+ // Note: The returned ProjectContext contains a shared pointer into
+ // this factory's CobaltRegistry. It does not matter whether or not this
+ // ProjectContextFactory remains alive.
std::unique_ptr<encoder::ProjectContext> NewLegacyProjectContext(
uint32_t customer_id, uint32_t project_id);
@@ -112,8 +120,9 @@
// unique Cobalt 0.1 project contained in the factory's CobaltRegistry.
// Returns nullptr otherwise.
//
- // This ProjectContextFactory must remain alive as long as the returned
- // ProjectContext is being used.
+ // Note: The returned ProjectContext contains a shared pointer into
+ // this factory's CobaltRegistry. It does not matter whether or not this
+ // ProjectContextFactory remains alive.
std::unique_ptr<encoder::ProjectContext> NewSingleLegacyProjectContext();
private:
diff --git a/logger/project_context_factory_test.cc b/logger/project_context_factory_test.cc
index aecd141..b992d59 100644
--- a/logger/project_context_factory_test.cc
+++ b/logger/project_context_factory_test.cc
@@ -109,11 +109,9 @@
EXPECT_FALSE(factory.is_valid());
EXPECT_FALSE(factory.is_single_project());
EXPECT_FALSE(factory.is_single_legacy_project());
- EXPECT_EQ(nullptr,
- factory.NewProjectContext("Customer11", "Project11").get());
- EXPECT_EQ(nullptr,
- factory.NewProjectContext("Customer22", "Project22").get());
- EXPECT_EQ(nullptr, factory.NewSingleProjectContext().get());
+ EXPECT_EQ(nullptr, factory.NewProjectContext("Customer11", "Project11"));
+ EXPECT_EQ(nullptr, factory.NewProjectContext("Customer22", "Project22"));
+ EXPECT_EQ(nullptr, factory.TakeSingleProjectContext());
EXPECT_EQ(nullptr, factory.NewLegacyProjectContext(11, 11).get());
EXPECT_EQ(nullptr, factory.NewSingleLegacyProjectContext().get());
}
@@ -139,7 +137,7 @@
factory.NewProjectContext("Customer22", "Project22").get());
// Registry A does not contain a single Cobalt 1.0 project.
- EXPECT_EQ(nullptr, factory.NewSingleProjectContext().get());
+ EXPECT_EQ(nullptr, factory.TakeSingleProjectContext());
// Registry A does contain some Cobalt 0.1 project data.
EXPECT_NE(nullptr, factory.NewLegacyProjectContext(11, 11).get());
@@ -182,22 +180,21 @@
EXPECT_NE(nullptr,
factory.NewProjectContext("Customer22", "Project22").get());
- // Registry B does contain a single Cobalt 1.0 project.
- EXPECT_NE(nullptr, factory.NewSingleProjectContext().get());
-
// Registry B does not contain any Cobalt 0.1 project data.
EXPECT_EQ(nullptr, factory.NewLegacyProjectContext(11, 11).get());
EXPECT_EQ(nullptr, factory.NewSingleLegacyProjectContext().get());
+ // Registry B does contain a single Cobalt 1.0 project.
+ auto context = factory.TakeSingleProjectContext();
+ EXPECT_NE(nullptr, context);
+
// The single Cobalt 1.0 project contains metric 22 but not metric 11.
- auto context = factory.NewSingleProjectContext();
EXPECT_EQ(nullptr, context->GetMetric("Metric11"));
EXPECT_NE(nullptr, context->GetMetric("Metric22"));
- // Cobalt 1.0 project 22 contains metric 22 but not metric 11.
- context = factory.NewProjectContext("Customer22", "Project22");
- EXPECT_EQ(nullptr, context->GetMetric("Metric11"));
- EXPECT_NE(nullptr, context->GetMetric("Metric22"));
+ // The data has been removed from the factory.
+ EXPECT_EQ(nullptr, factory.NewProjectContext("Customer22", "Project22"));
+ EXPECT_FALSE(factory.is_valid());
}
TEST(ProjectContextFactoryTest, RegistryC) {
@@ -219,7 +216,7 @@
factory.NewProjectContext("Customer22", "Project22").get());
// Registry C does not contain only a single Cobalt 0.1 project.
- EXPECT_EQ(nullptr, factory.NewSingleProjectContext().get());
+ EXPECT_EQ(nullptr, factory.TakeSingleProjectContext());
// Registry C contains Cobalt 0.1 project 11 but not project 22.
EXPECT_NE(nullptr, factory.NewLegacyProjectContext(11, 11).get());
