tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rename Find[Vertex|Edge][Checked]

Browse Source 
Summary:
Find[Vertex|Edge] -> Find[Vertex|Edge]Optional
Find[Vertex|Edge]Checked -> Find[Vertex|Edge]

In some places change old code that finds-optional and immediately checks
to use the checked functionality.

It seems that in all the src/ stuff optional finds are no loger used,
only in tests, but there they are used extensively so I don't feel those
functions should be removed.

Reviewers: dgleich

Reviewed By: dgleich

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1309
This commit is contained in:
florijan 2018-03-21 10:19:23 +01:00
parent c35d7fbec6
commit a39602093d
8 changed files with 85 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/database/graph_db_accessor.cpp
View File

@ -98,7 +98,7 @@ VertexAccessor GraphDbAccessor::InsertVertexIntoRemote(
return VertexAccessor({gid, worker_id}, *this);
}
std::experimental::optional<VertexAccessor> GraphDbAccessor::FindVertex(
std::experimental::optional<VertexAccessor> GraphDbAccessor::FindVertexOptional(
gid::Gid gid, bool current_state) {
VertexAccessor record_accessor(db_.storage().LocalAddress<Vertex>(gid),
*this);
@ -107,14 +107,13 @@ std::experimental::optional<VertexAccessor> GraphDbAccessor::FindVertex(
return record_accessor;
}
VertexAccessor GraphDbAccessor::FindVertexChecked(gid::Gid gid,
bool current_state) {
auto found = FindVertex(gid, current_state);
VertexAccessor GraphDbAccessor::FindVertex(gid::Gid gid, bool current_state) {
auto found = FindVertexOptional(gid, current_state);
CHECK(found) << "Unable to find vertex for id: " << gid;
return *found;
}
std::experimental::optional<EdgeAccessor> GraphDbAccessor::FindEdge(
std::experimental::optional<EdgeAccessor> GraphDbAccessor::FindEdgeOptional(
gid::Gid gid, bool current_state) {
EdgeAccessor record_accessor(db_.storage().LocalAddress<Edge>(gid), *this);
if (!record_accessor.Visible(transaction(), current_state))
@ -122,9 +121,8 @@ std::experimental::optional<EdgeAccessor> GraphDbAccessor::FindEdge(
return record_accessor;
}
EdgeAccessor GraphDbAccessor::FindEdgeChecked(gid::Gid gid,
bool current_state) {
auto found = FindEdge(gid, current_state);
EdgeAccessor GraphDbAccessor::FindEdge(gid::Gid gid, bool current_state) {
auto found = FindEdgeOptional(gid, current_state);
CHECK(found) << "Unable to find edge for id: " << gid;
return *found;
}

36
src/database/graph_db_accessor.hpp
View File

@ -119,11 +119,21 @@ class GraphDbAccessor {
* deletions performed in the current transaction+command are not
* ignored).
*/
std::experimental::optional<VertexAccessor> FindVertex(gid::Gid gid,
bool current_state);
std::experimental::optional<VertexAccessor> FindVertexOptional(
gid::Gid gid, bool current_state);
/** Like `FindVertex`, but performs a CHECK that the result is found. */
VertexAccessor FindVertexChecked(gid::Gid gid, bool current_state);
/**
* Obtains the vertex for the given ID. If there is no vertex for the given
* ID, or it's not visible to this accessor's transaction, MG is crashed
* using a CHECK.
*
* @param gid - The GID of the sought vertex.
* @param current_state If true then the graph state for the
* current transaction+command is returned (insertions, updates and
* deletions performed in the current transaction+command are not
* ignored).
*/
VertexAccessor FindVertex(gid::Gid gid, bool current_state);
/**
* Returns iterable over accessors to all the vertices in the graph
@ -324,11 +334,21 @@ class GraphDbAccessor {
* deletions performed in the current transaction+command are not
* ignored).
*/
std::experimental::optional<EdgeAccessor> FindEdge(gid::Gid gid,
bool current_state);
std::experimental::optional<EdgeAccessor> FindEdgeOptional(
gid::Gid gid, bool current_state);
/** Like `FindEdge`, but performs a CHECK that the result is found. */
EdgeAccessor FindEdgeChecked(gid::Gid gid, bool current_state);
/**
* Obtains the edge for the given ID. If there is no edge for the given
* ID, or it's not visible to this accessor's transaction, MG is crashed
* using a CHECK.
*
* @param gid - The GID of the sought edge.
* @param current_state If true then the graph state for the
* current transaction+command is returned (insertions, updates and
* deletions performed in the current transaction+command are not
* ignored).
*/
EdgeAccessor FindEdge(gid::Gid gid, bool current_state);
/**
* Returns iterable over accessors to all the edges in the graph

22
src/database/state_delta.cpp
View File

@ -359,9 +359,7 @@ void StateDelta::Apply(GraphDbAccessor &dba) const {
case Type::CREATE_EDGE: {
auto from = dba.FindVertex(vertex_from_id, true);
auto to = dba.FindVertex(vertex_to_id, true);
DCHECK(from) << "Failed to find vertex.";
DCHECK(to) << "Failed to find vertex.";
dba.InsertEdge(*from, *to, dba.EdgeType(edge_type_name), edge_id);
dba.InsertEdge(from, to, dba.EdgeType(edge_type_name), edge_id);
break;
}
case Type::ADD_OUT_EDGE:
@ -371,38 +369,32 @@ void StateDelta::Apply(GraphDbAccessor &dba) const {
LOG(FATAL) << "Partial edge creation/deletion not yet supported in Apply";
case Type::SET_PROPERTY_VERTEX: {
auto vertex = dba.FindVertex(vertex_id, true);
DCHECK(vertex) << "Failed to find vertex.";
vertex->PropsSet(dba.Property(property_name), value);
vertex.PropsSet(dba.Property(property_name), value);
break;
}
case Type::SET_PROPERTY_EDGE: {
auto edge = dba.FindEdge(edge_id, true);
DCHECK(edge) << "Failed to find edge.";
edge->PropsSet(dba.Property(property_name), value);
edge.PropsSet(dba.Property(property_name), value);
break;
}
case Type::ADD_LABEL: {
auto vertex = dba.FindVertex(vertex_id, true);
DCHECK(vertex) << "Failed to find vertex.";
vertex->add_label(dba.Label(label_name));
vertex.add_label(dba.Label(label_name));
break;
}
case Type::REMOVE_LABEL: {
auto vertex = dba.FindVertex(vertex_id, true);
DCHECK(vertex) << "Failed to find vertex.";
vertex->remove_label(dba.Label(label_name));
vertex.remove_label(dba.Label(label_name));
break;
}
case Type::REMOVE_VERTEX: {
auto vertex = dba.FindVertex(vertex_id, true);
DCHECK(vertex) << "Failed to find vertex.";
dba.DetachRemoveVertex(*vertex);
dba.DetachRemoveVertex(vertex);
break;
}
case Type::REMOVE_EDGE: {
auto edge = dba.FindEdge(edge_id, true);
DCHECK(edge) << "Failed to find edge.";
dba.RemoveEdge(*edge);
dba.RemoveEdge(edge);
break;
}
case Type::BUILD_INDEX: {

4
src/distributed/remote_data_rpc_server.cpp
View File

@ -11,7 +11,7 @@ RemoteDataRpcServer::RemoteDataRpcServer(database::GraphDb &db,
: db_(db), rpc_server_(server) {
rpc_server_.Register<RemoteVertexRpc>([this](const RemoteVertexReq &req) {
database::GraphDbAccessor dba(db_, req.member.tx_id);
auto vertex = dba.FindVertexChecked(req.member.gid, false);
auto vertex = dba.FindVertex(req.member.gid, false);
CHECK(vertex.GetOld())
<< "Old record must exist when sending vertex by RPC";
return std::make_unique<RemoteVertexRes>(vertex.GetOld(), db_.WorkerId());
@ -19,7 +19,7 @@ RemoteDataRpcServer::RemoteDataRpcServer(database::GraphDb &db,
rpc_server_.Register<RemoteEdgeRpc>([this](const RemoteEdgeReq &req) {
database::GraphDbAccessor dba(db_, req.member.tx_id);
auto edge = dba.FindEdgeChecked(req.member.gid, false);
auto edge = dba.FindEdge(req.member.gid, false);
CHECK(edge.GetOld()) << "Old record must exist when sending edge by RPC";
return std::make_unique<RemoteEdgeRes>(edge.GetOld(), db_.WorkerId());
});

4
src/distributed/remote_updates_rpc_server.cpp
View File

@ -349,14 +349,14 @@ template <>
VertexAccessor
RemoteUpdatesRpcServer::TransactionUpdates<VertexAccessor>::FindAccessor(
gid::Gid gid) {
return db_accessor_.FindVertexChecked(gid, false);
return db_accessor_.FindVertex(gid, false);
}
template <>
EdgeAccessor
RemoteUpdatesRpcServer::TransactionUpdates<EdgeAccessor>::FindAccessor(
gid::Gid gid) {
return db_accessor_.FindEdgeChecked(gid, false);
return db_accessor_.FindEdge(gid, false);
}
} // namespace distributed

56
tests/unit/distributed_updates.cpp
View File

@ -76,7 +76,7 @@ TEST_F(DistributedGraphDbTest, CreateVertex) {
}
{
database::GraphDbAccessor dba{worker(2)};
auto v = dba.FindVertex(gid, false);
auto v = dba.FindVertexOptional(gid, false);
ASSERT_TRUE(v);
}
}
@ -95,7 +95,7 @@ TEST_F(DistributedGraphDbTest, CreateVertexWithUpdate) {
}
{
database::GraphDbAccessor dba{worker(2)};
auto v = dba.FindVertex(gid, false);
auto v = dba.FindVertexOptional(gid, false);
ASSERT_TRUE(v);
EXPECT_EQ(v->PropsAt(prop).Value<int64_t>(), 42);
}
@ -124,7 +124,7 @@ TEST_F(DistributedGraphDbTest, CreateVertexWithData) {
}
{
database::GraphDbAccessor dba{worker(2)};
auto v = dba.FindVertex(gid, false);
auto v = dba.FindVertexOptional(gid, false);
ASSERT_TRUE(v);
// Check remote data after commit.
EXPECT_TRUE(v->has_label(l1));
@ -150,7 +150,7 @@ TEST_F(DistributedGraphDbTest, UpdateVertexRemoteAndLocal) {
{
database::GraphDbAccessor dba0{master()};
database::GraphDbAccessor dba1{worker(1), dba0.transaction_id()};
auto v_local = dba1.FindVertexChecked(gid, false);
auto v_local = dba1.FindVertex(gid, false);
auto v_remote = VertexAccessor(storage::VertexAddress(gid, 1), dba0);
v_remote.add_label(l2);
@ -167,7 +167,7 @@ TEST_F(DistributedGraphDbTest, AddSameLabelRemoteAndLocal) {
{
database::GraphDbAccessor dba0{master()};
database::GraphDbAccessor dba1{worker(1), dba0.transaction_id()};
auto v_local = dba1.FindVertexChecked(v_address.gid(), false);
auto v_local = dba1.FindVertex(v_address.gid(), false);
auto v_remote = VertexAccessor(v_address, dba0);
auto l1 = dba1.Label("label");
v_remote.add_label(l1);
@ -178,7 +178,7 @@ TEST_F(DistributedGraphDbTest, AddSameLabelRemoteAndLocal) {
{
database::GraphDbAccessor dba0{master()};
database::GraphDbAccessor dba1{worker(1), dba0.transaction_id()};
auto v = dba1.FindVertexChecked(v_address.gid(), false);
auto v = dba1.FindVertex(v_address.gid(), false);
EXPECT_EQ(v.labels().size(), 1);
}
}
@ -207,10 +207,10 @@ TEST_F(DistributedGraphDbTest, DeleteVertexRemoteCommit) {
database::GraphDbAccessor dba1{worker(1), dba0.transaction_id()};
auto v_remote = VertexAccessor(v_address, dba0);
dba0.RemoveVertex(v_remote);
EXPECT_TRUE(dba1.FindVertex(v_address.gid(), true));
EXPECT_TRUE(dba1.FindVertexOptional(v_address.gid(), true));
EXPECT_EQ(worker(1).remote_updates_server().Apply(dba0.transaction_id()),
distributed::RemoteUpdateResult::DONE);
EXPECT_FALSE(dba1.FindVertex(v_address.gid(), true));
EXPECT_FALSE(dba1.FindVertexOptional(v_address.gid(), true));
}
TEST_F(DistributedGraphDbTest, DeleteVertexRemoteBothDelete) {
@ -218,13 +218,13 @@ TEST_F(DistributedGraphDbTest, DeleteVertexRemoteBothDelete) {
{
database::GraphDbAccessor dba0{master()};
database::GraphDbAccessor dba1{worker(1), dba0.transaction_id()};
auto v_local = dba1.FindVertexChecked(v_address.gid(), false);
auto v_local = dba1.FindVertex(v_address.gid(), false);
auto v_remote = VertexAccessor(v_address, dba0);
EXPECT_TRUE(dba1.RemoveVertex(v_local));
EXPECT_TRUE(dba0.RemoveVertex(v_remote));
EXPECT_EQ(worker(1).remote_updates_server().Apply(dba0.transaction_id()),
distributed::RemoteUpdateResult::DONE);
EXPECT_FALSE(dba1.FindVertex(v_address.gid(), true));
EXPECT_FALSE(dba1.FindVertexOptional(v_address.gid(), true));
}
}
@ -239,13 +239,13 @@ TEST_F(DistributedGraphDbTest, DeleteVertexRemoteStillConnected) {
dba0.RemoveVertex(v_remote);
EXPECT_EQ(worker(1).remote_updates_server().Apply(dba0.transaction_id()),
distributed::RemoteUpdateResult::UNABLE_TO_DELETE_VERTEX_ERROR);
EXPECT_TRUE(dba1.FindVertex(v_address.gid(), true));
EXPECT_TRUE(dba1.FindVertexOptional(v_address.gid(), true));
}
{
database::GraphDbAccessor dba0{master()};
database::GraphDbAccessor dba1{worker(1), dba0.transaction_id()};
auto e_local = dba1.FindEdgeChecked(e_address.gid(), false);
auto v_local = dba1.FindVertexChecked(v_address.gid(), false);
auto e_local = dba1.FindEdge(e_address.gid(), false);
auto v_local = dba1.FindVertex(v_address.gid(), false);
auto v_remote = VertexAccessor(v_address, dba0);
dba1.RemoveEdge(e_local);
@ -253,7 +253,7 @@ TEST_F(DistributedGraphDbTest, DeleteVertexRemoteStillConnected) {
EXPECT_EQ(worker(1).remote_updates_server().Apply(dba0.transaction_id()),
distributed::RemoteUpdateResult::DONE);
EXPECT_FALSE(dba1.FindVertex(v_address.gid(), true));
EXPECT_FALSE(dba1.FindVertexOptional(v_address.gid(), true));
}
}
@ -302,8 +302,8 @@ TEST_F(DistributedDetachDeleteTest, VertexCycle) {
Run(w1_a,
[this, e_address](
std::vector<std::reference_wrapper<database::GraphDbAccessor>> &dba) {
EXPECT_FALSE(dba[1].get().FindVertex(w1_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdge(e_address.gid(), true));
EXPECT_FALSE(dba[1].get().FindVertexOptional(w1_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdgeOptional(e_address.gid(), true));
});
}
@ -314,18 +314,18 @@ TEST_F(DistributedDetachDeleteTest, TwoVerticesDifferentWorkers) {
Run(w1_a,
[this, e_address](
std::vector<std::reference_wrapper<database::GraphDbAccessor>> &dba) {
EXPECT_FALSE(dba[1].get().FindVertex(w1_a.gid(), true));
EXPECT_TRUE(dba[2].get().FindVertex(w2_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdge(e_address.gid(), true));
EXPECT_FALSE(dba[1].get().FindVertexOptional(w1_a.gid(), true));
EXPECT_TRUE(dba[2].get().FindVertexOptional(w2_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdgeOptional(e_address.gid(), true));
});
// Delete to
Run(w2_a,
[this, e_address](
std::vector<std::reference_wrapper<database::GraphDbAccessor>> &dba) {
EXPECT_TRUE(dba[1].get().FindVertex(w1_a.gid(), true));
EXPECT_FALSE(dba[2].get().FindVertex(w2_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdge(e_address.gid(), true));
EXPECT_TRUE(dba[1].get().FindVertexOptional(w1_a.gid(), true));
EXPECT_FALSE(dba[2].get().FindVertexOptional(w2_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdgeOptional(e_address.gid(), true));
});
}
@ -336,18 +336,18 @@ TEST_F(DistributedDetachDeleteTest, TwoVerticesSameWorkers) {
Run(w1_a,
[this, e_address](
std::vector<std::reference_wrapper<database::GraphDbAccessor>> &dba) {
EXPECT_FALSE(dba[1].get().FindVertex(w1_a.gid(), true));
EXPECT_TRUE(dba[1].get().FindVertex(w1_b.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdge(e_address.gid(), true));
EXPECT_FALSE(dba[1].get().FindVertexOptional(w1_a.gid(), true));
EXPECT_TRUE(dba[1].get().FindVertexOptional(w1_b.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdgeOptional(e_address.gid(), true));
});
// Delete to
Run(w1_b,
[this, e_address](
std::vector<std::reference_wrapper<database::GraphDbAccessor>> &dba) {
EXPECT_TRUE(dba[1].get().FindVertex(w1_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindVertex(w1_b.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdge(e_address.gid(), true));
EXPECT_TRUE(dba[1].get().FindVertexOptional(w1_a.gid(), true));
EXPECT_FALSE(dba[1].get().FindVertexOptional(w1_b.gid(), true));
EXPECT_FALSE(dba[1].get().FindEdgeOptional(e_address.gid(), true));
});
}

8
tests/unit/durability.cpp
View File

@ -46,11 +46,11 @@ class DbGenerator {
}
EdgeAccessor RandomEdge(bool remove_from_ids = false) {
return *dba_.FindEdge(RandomElement(edge_ids_, remove_from_ids), true);
return dba_.FindEdge(RandomElement(edge_ids_, remove_from_ids), true);
}
VertexAccessor RandomVertex(bool remove_from_ids = false) {
return *dba_.FindVertex(RandomElement(vertex_ids_, remove_from_ids), true);
return dba_.FindVertex(RandomElement(vertex_ids_, remove_from_ids), true);
}
VertexAccessor InsertVertex() {
@ -197,7 +197,7 @@ void CompareDbs(database::GraphDb &a, database::GraphDb &b) {
int vertices_a_count = 0;
for (auto v_a : dba_a.Vertices(false)) {
vertices_a_count++;
auto v_b = dba_b.FindVertex(v_a.gid(), false);
auto v_b = dba_b.FindVertexOptional(v_a.gid(), false);
ASSERT_TRUE(v_b) << "Vertex not found, id: " << v_a.gid();
ASSERT_EQ(v_a.labels().size(), v_b->labels().size());
std::vector<std::string> v_a_labels;
@ -216,7 +216,7 @@ void CompareDbs(database::GraphDb &a, database::GraphDb &b) {
int edges_a_count = 0;
for (auto e_a : dba_a.Edges(false)) {
edges_a_count++;
auto e_b = dba_b.FindEdge(e_a.gid(), false);
auto e_b = dba_b.FindEdgeOptional(e_a.gid(), false);
ASSERT_TRUE(e_b);
ASSERT_TRUE(e_b) << "Edge not found, id: " << e_a.gid();
EXPECT_EQ(dba_a.EdgeTypeName(e_a.EdgeType()),

16
tests/unit/state_delta.cpp
View File

@ -16,7 +16,7 @@ TEST(StateDelta, CreateVertex) {
}
{
database::GraphDbAccessor dba(db);
auto vertex = dba.FindVertex(gid0, false);
auto vertex = dba.FindVertexOptional(gid0, false);
EXPECT_TRUE(vertex);
}
}
@ -39,7 +39,7 @@ TEST(StateDelta, RemoveVertex) {
}
{
database::GraphDbAccessor dba(db);
auto vertex = dba.FindVertex(gid0, false);
auto vertex = dba.FindVertexOptional(gid0, false);
EXPECT_FALSE(vertex);
}
}
@ -65,7 +65,7 @@ TEST(StateDelta, CreateEdge) {
}
{
database::GraphDbAccessor dba(db);
auto edge = dba.FindEdge(gid2, false);
auto edge = dba.FindEdgeOptional(gid2, false);
EXPECT_TRUE(edge);
}
}
@ -91,7 +91,7 @@ TEST(StateDelta, RemoveEdge) {
}
{
database::GraphDbAccessor dba(db);
auto edge = dba.FindEdge(gid2, false);
auto edge = dba.FindEdgeOptional(gid2, false);
EXPECT_FALSE(edge);
}
}
@ -114,7 +114,7 @@ TEST(StateDelta, AddLabel) {
}
{
database::GraphDbAccessor dba(db);
auto vertex = dba.FindVertex(gid0, false);
auto vertex = dba.FindVertexOptional(gid0, false);
EXPECT_TRUE(vertex);
auto labels = vertex->labels();
EXPECT_EQ(labels.size(), 1);
@ -141,7 +141,7 @@ TEST(StateDelta, RemoveLabel) {
}
{
database::GraphDbAccessor dba(db);
auto vertex = dba.FindVertex(gid0, false);
auto vertex = dba.FindVertexOptional(gid0, false);
EXPECT_TRUE(vertex);
auto labels = vertex->labels();
EXPECT_EQ(labels.size(), 0);
@ -167,7 +167,7 @@ TEST(StateDelta, SetPropertyVertex) {
}
{
database::GraphDbAccessor dba(db);
auto vertex = dba.FindVertex(gid0, false);
auto vertex = dba.FindVertexOptional(gid0, false);
EXPECT_TRUE(vertex);
auto prop = vertex->PropsAt(dba.Property("property"));
EXPECT_EQ(prop.Value<int64_t>(), 2212);
@ -197,7 +197,7 @@ TEST(StateDelta, SetPropertyEdge) {
}
{
database::GraphDbAccessor dba(db);
auto edge = dba.FindEdge(gid2, false);
auto edge = dba.FindEdgeOptional(gid2, false);
EXPECT_TRUE(edge);
auto prop = edge->PropsAt(dba.Property("property"));
EXPECT_EQ(prop.Value<int64_t>(), 2212);