Skip to content

Commit

Permalink
Implement StatusOr<void>. (#1706)
Browse files Browse the repository at this point in the history
This is just an specialization for routines that return `void`. One
could just return `Status` in this case, but generic code is easier to
write if we have this specialization.
  • Loading branch information
coryan authored Dec 18, 2018
1 parent 3912f38 commit b3eeb2d
Show file tree
Hide file tree
Showing 3 changed files with 223 additions and 0 deletions.
126 changes: 126 additions & 0 deletions google/cloud/storage/status_or.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,132 @@ class StatusOr final {
T value_;
};

/**
* `StatusOr<void>` is used to return an error or nothing.
*
* `StatusOr<T>` does not work for `T = void` because some of the member
* functions (`StatusOr<T>(T)`) make no sense for `void`. Likewise, the class
* cannot contain an object of type `void` because there is no such thing.
*
* The application typically calls an API in the library and must check the
* returned error status:
*
* @code
* namespace gcs = google::cloud::storage;
* void AppCode(gcs::noex::Client client) {
* gcs::StatusOr<void> delete_err = client.DeleteBucket("my-bucket-name");
* if (not delete_err.ok()) {
* std::cerr << "Error in DeleteBucket: " << meta_err.status()
* << std::endl;
* return;
* }
* // Do useful work here.
* }
* @endcode
*
* Note that the storage client retries most requests for you, resending the
* request after an error is probably not useful. You should consider changing
* the retry policies instead.
*/
template <>
class StatusOr<void> final {
public:
/**
* Initializes with an error status (UNKNOWN).
*
* TODO(#548) - currently storage::Status does not define the status codes,
* they are simply integers, usually HTTP status codes. We need to map to
* the well-defined set of status codes.
*/
StatusOr() : StatusOr(Status(500, "UNKNOWN")) {}

/**
* Creates a new `StatusOr<void>` holding the status @p rhs.
*
* When `rhs.ok() == true` the object is treated as if it held a `void` value.
*
* @param rhs the status to initialize the object.
*/
// NOLINTNEXTLINE(google-explicit-constructor)
StatusOr(Status rhs) : status_(std::move(rhs)) {}

bool ok() const { return status_.ok(); }
explicit operator bool() const { return ok(); }

//@{
/**
* @name Deference operators.
*
* These are provided mostly so generic code can use `StatusOr<void>` just
* like `StatusOr<T>`.
*/
void operator*() & {}
void operator*() const& {}
void operator*() && {}
void operator*() const&& {}
//@}

//@{
/**
* @name Member access operators.
*
* These are provided mostly so generic code can use `StatusOr<void>` just
* like `StatusOr<T>`.
*/
void* operator->() & { return nullptr; }
void const* operator->() const& { return nullptr; }
//@}

//@{
/**
* @name Value accessors.
*
* @return All these member functions return a (properly ref and
* const-qualified) reference to the underlying value.
*
* @throws `RuntimeStatusError` with the contents of `status()` if the object
* does not contain a value, i.e., if `ok() == false`.
*/
void value() & { CheckHasValue(); }

void value() const& { CheckHasValue(); }

void value() && { CheckHasValue(); }

void value() const&& { CheckHasValue(); }
//@}

//@{
/**
* @name Status accessors.
*
* @return All these member functions return the (properly ref and
* const-qualified) status. If the object contains a value then
* `status().ok() == true`.
*/
Status& status() & { return status_; }
Status const& status() const& { return status_; }
Status&& status() && { return std::move(status_); }
Status const&& status() const&& { return std::move(status_); }
//@}

private:
void CheckHasValue() const& {
if (not ok()) {
internal::ThrowStatus(Status(status_));
}
}

// When possible, do not copy the status.
void CheckHasValue() && {
if (not ok()) {
internal::ThrowStatus(std::move(status_));
}
}

Status status_;
};

} // namespace STORAGE_CLIENT_NS
} // namespace storage
} // namespace cloud
Expand Down
78 changes: 78 additions & 0 deletions google/cloud/storage/status_or_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,84 @@ TEST(StatusOrTest, ValueConstArrow) {
EXPECT_EQ(std::string("42"), actual->c_str());
}

TEST(StatusOrVoidTest, DefaultConstructor) {
StatusOr<void> actual;
EXPECT_FALSE(actual.ok());
EXPECT_FALSE(actual.status().ok());
}

TEST(StatusOrVoidTest, StatusConstructorNormal) {
StatusOr<void> actual(Status(404, "NOT FOUND", "It was there yesterday!"));
EXPECT_FALSE(actual.ok());
EXPECT_EQ(404, actual.status().status_code());
EXPECT_EQ("NOT FOUND", actual.status().error_message());
EXPECT_EQ("It was there yesterday!", actual.status().error_details());
}

TEST(StatusOrVoidTest, ValueConstructor) {
StatusOr<void> actual(Status{});
EXPECT_TRUE(actual.ok());
testing_util::ExpectNoException([&] { actual.value(); });
testing_util::ExpectNoException([&] { std::move(actual).value(); });
}

TEST(StatusOrVoidTest, ValueConstAccessors) {
StatusOr<void> const actual(Status{});
EXPECT_TRUE(actual.ok());
testing_util::ExpectNoException([&] { actual.value(); });
testing_util::ExpectNoException([&] { std::move(actual).value(); });
}

TEST(StatusOrVoidTest, ValueAccessorNonConstThrows) {
StatusOr<void> actual(Status(500, "BAD"));

testing_util::ExpectException<RuntimeStatusError>(
[&] { actual.value(); },
[&](RuntimeStatusError const& ex) {
EXPECT_EQ(500, ex.status().status_code());
EXPECT_EQ("BAD", ex.status().error_message());
},
"exceptions are disabled: BAD \\[500\\]"
);

testing_util::ExpectException<RuntimeStatusError>(
[&] { std::move(actual).value(); },
[&](RuntimeStatusError const& ex) {
EXPECT_EQ(500, ex.status().status_code());
EXPECT_EQ("BAD", ex.status().error_message());
},
"exceptions are disabled: BAD \\[500\\]"
);
}

TEST(StatusOrVoidTest, ValueAccessorConstThrows) {
StatusOr<void> actual(Status(500, "BAD"));

testing_util::ExpectException<RuntimeStatusError>(
[&] { actual.value(); },
[&](RuntimeStatusError const& ex) {
EXPECT_EQ(500, ex.status().status_code());
EXPECT_EQ("BAD", ex.status().error_message());
},
"exceptions are disabled: BAD \\[500\\]"
);

testing_util::ExpectException<RuntimeStatusError>(
[&] { std::move(actual).value(); },
[&](RuntimeStatusError const& ex) {
EXPECT_EQ(500, ex.status().status_code());
EXPECT_EQ("BAD", ex.status().error_message());
},
"exceptions are disabled: BAD \\[500\\]"
);
}

TEST(StatusOrVoidTest, StatusConstAccessors) {
StatusOr<void> const actual(Status(500, "BAD"));
EXPECT_EQ(500, actual.status().status_code());
EXPECT_EQ(500, std::move(actual).status().status_code());
}

} // namespace
} // namespace STORAGE_CLIENT_NS
} // namespace storage
Expand Down
19 changes: 19 additions & 0 deletions google/cloud/testing_util/expect_exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,25 @@ void ExpectException(
EXPECT_DEATH_IF_SUPPORTED(expression(), expected_message);
#endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
}

/**
* Verify that an expression does not throw.
*
* Testing `void` expressions is tedious because `EXPECT_NO_THROW` does not
* compile when exceptions are disabled. Writing `expression()` in a test does
* detect exceptions, but does not express the intent.
*
* @param expression the expression (typically a lambda) that we want to verify
* does not throw.
*/
inline void ExpectNoException(std::function<void()> const& expression) {
#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
EXPECT_NO_THROW(expression());
#else
EXPECT_NO_FATAL_FAILURE(expression());
#endif // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
}

} // namespace testing_util
} // namespace GOOGLE_CLOUD_CPP_NS
} // namespace cloud
Expand Down

0 comments on commit b3eeb2d

Please sign in to comment.