Make consistent set of get and getTry methods on SemiFuture.

Summary: Complete set of get and getVia methods including addition of a result operation on FutureBase that provides functionality of the old getTry on Future by returning a Try by reference.

Reviewed By: yfeldblum

Differential Revision: D6640056

fbshipit-source-id: 3ac01f7bc4b128e641f08d9a99280a18ffce82f9

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index 25f8f464f0d1a2e5af1726cfa5bb5457366b4c39..56b98410abcfe59d24cfcdbe89caecf08a0d8ba2 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #pragma once
 
 #include <algorithm>
@@ -174,30 +173,50 @@ FutureBase<T>::~FutureBase() {
 
 template <class T>
 T& FutureBase<T>::value() & {
+  return result().value();
+}
+
+template <class T>
+T const& FutureBase<T>::value() const& {
+  return result().value();
+}
+
+template <class T>
+T&& FutureBase<T>::value() && {
+  return std::move(result().value());
+}
+
+template <class T>
+T const&& FutureBase<T>::value() const&& {
+  return std::move(result().value());
+}
+
+template <class T>
+Try<T>& FutureBase<T>::result() & {
   throwIfInvalid();
 
-  return core_->getTry().value();
+  return core_->getTry();
 }
 
 template <class T>
-T const& FutureBase<T>::value() const& {
+Try<T> const& FutureBase<T>::result() const& {
   throwIfInvalid();
 
-  return core_->getTry().value();
+  return core_->getTry();
 }
 
 template <class T>
-T&& FutureBase<T>::value() && {
+Try<T>&& FutureBase<T>::result() && {
   throwIfInvalid();
 
-  return std::move(core_->getTry().value());
+  return std::move(core_->getTry());
 }
 
 template <class T>
-T const&& FutureBase<T>::value() const&& {
+Try<T> const&& FutureBase<T>::result() const&& {
   throwIfInvalid();
 
-  return std::move(core_->getTry().value());
+  return std::move(core_->getTry());
 }
 
 template <class T>
@@ -1443,7 +1462,7 @@ SemiFuture<T>&& SemiFuture<T>::waitVia(DrivableExecutor* e) && {
 
 template <class T>
 T SemiFuture<T>::get() && {
-  return std::move(wait().value());
+  return std::move(wait()).value();
 }
 
 template <class T>
@@ -1458,19 +1477,27 @@ T SemiFuture<T>::get(Duration dur) && {
 
 template <class T>
 Try<T> SemiFuture<T>::getTry() && {
-  wait();
-  return std::move(this->core_->getTry());
+  return std::move(wait()).result();
+}
+
+template <class T>
+Try<T> SemiFuture<T>::getTry(Duration dur) && {
+  wait(dur);
+  if (this->isReady()) {
+    return std::move(this->result());
+  } else {
+    throwTimedOut();
+  }
 }
 
 template <class T>
 T SemiFuture<T>::getVia(DrivableExecutor* e) && {
-  return std::move(waitVia(e).value());
+  return std::move(waitVia(e)).value();
 }
 
 template <class T>
 Try<T> SemiFuture<T>::getTryVia(DrivableExecutor* e) && {
-  waitVia(e);
-  return std::move(this->core_->getTry());
+  return std::move(waitVia(e)).result();
 }
 
 template <class T>
@@ -1526,9 +1553,7 @@ T Future<T>::get(Duration dur) {
 
 template <class T>
 Try<T>& Future<T>::getTry() {
-  throwIfInvalid();
-
-  return this->core_->getTry();
+  return result();
 }
 
 template <class T>

diff --git a/folly/futures/Future.h b/folly/futures/Future.h
index 4818e1ede2959a0010607adf3248370e59d67cb5..5b2dc82f7b44b6c8f9a4a910f1f17fdb0c6c1204 100644 (file)
--- a/folly/futures/Future.h
+++ b/folly/futures/Future.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #pragma once
 
 #include <algorithm>
@@ -97,6 +96,13 @@ class FutureBase {
   T&& value() &&;
   T const&& value() const&&;
 
+  /// Returns a reference to the try of the result. Throws as for value if
+  /// future is not valid.
+  Try<T>& result() &;
+  Try<T> const& result() const&;
+  Try<T>&& result() &&;
+  Try<T> const&& result() const&&;
+
   /** True when the result (or exception) is ready. */
   bool isReady() const;
 
@@ -238,6 +244,7 @@ class SemiFuture : private futures::detail::FutureBase<T> {
   using Base::raise;
   using Base::setCallback_;
   using Base::value;
+  using Base::result;
 
   SemiFuture& operator=(SemiFuture const&) = delete;
   SemiFuture& operator=(SemiFuture&&) noexcept;
@@ -256,6 +263,10 @@ class SemiFuture : private futures::detail::FutureBase<T> {
   /// Try of the value (moved out).
   Try<T> getTry() &&;
 
+  /// Block until the future is fulfilled, or until timed out. Returns the
+  /// Try of the value (moved out) or may throw a TimedOut exception.
+  Try<T> getTry(Duration dur) &&;
+
   /// Call e->drive() repeatedly until the future is fulfilled. Examples
   /// of DrivableExecutor include EventBase and ManualExecutor. Returns the
   /// value (moved out), or throws the exception.
@@ -419,6 +430,7 @@ class Future : private futures::detail::FutureBase<T> {
   using Base::raise;
   using Base::setCallback_;
   using Base::value;
+  using Base::result;
 
   static Future<T> makeEmpty(); // equivalent to moved-from
 

diff --git a/folly/futures/test/SemiFutureTest.cpp b/folly/futures/test/SemiFutureTest.cpp
index 1c44fe8dd4a3f1a3d891b23484903984874b3624..36942645297c4296b005146d7fa8a770d2978b9e 100644 (file)
--- a/folly/futures/test/SemiFutureTest.cpp
+++ b/folly/futures/test/SemiFutureTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #include <folly/Executor.h>
 #include <folly/Memory.h>
 #include <folly/Unit.h>
@@ -264,6 +263,70 @@ TEST(SemiFuture, MakeFutureFromSemiFutureLValue) {
   ASSERT_EQ(result, 42);
 }
 
+TEST(SemiFuture, SimpleGet) {
+  EventBase e2;
+  Promise<int> p;
+  auto sf = p.getSemiFuture();
+  p.setValue(3);
+  auto v = std::move(sf).get();
+  ASSERT_EQ(v, 3);
+}
+
+TEST(SemiFuture, SimpleGetTry) {
+  EventBase e2;
+  Promise<int> p;
+  auto sf = p.getSemiFuture();
+  p.setValue(3);
+  auto v = std::move(sf).getTry();
+  ASSERT_EQ(v.value(), 3);
+}
+
+TEST(SemiFuture, SimpleTimedGet) {
+  EventBase e2;
+  Promise<folly::Unit> p;
+  auto sf = p.getSemiFuture();
+  EXPECT_THROW(std::move(sf).get(std::chrono::seconds(1)), TimedOut);
+}
+
+TEST(SemiFuture, SimpleTimedGetTry) {
+  EventBase e2;
+  Promise<folly::Unit> p;
+  auto sf = p.getSemiFuture();
+  EXPECT_THROW(std::move(sf).getTry(std::chrono::seconds(1)), TimedOut);
+}
+
+TEST(SemiFuture, SimpleValue) {
+  EventBase e2;
+  Promise<int> p;
+  auto sf = p.getSemiFuture();
+  p.setValue(3);
+  auto v = std::move(sf).value();
+  ASSERT_EQ(v, 3);
+}
+
+TEST(SemiFuture, SimpleValueThrow) {
+  EventBase e2;
+  Promise<folly::Unit> p;
+  auto sf = p.getSemiFuture();
+  EXPECT_THROW(std::move(sf).value(), FutureNotReady);
+}
+
+TEST(SemiFuture, SimpleResult) {
+  EventBase e2;
+  Promise<int> p;
+  auto sf = p.getSemiFuture();
+  p.setValue(3);
+  auto v = std::move(sf).result();
+  ASSERT_EQ(v.value(), 3);
+}
+
+TEST(SemiFuture, SimpleResultThrow) {
+  EventBase e2;
+  Promise<folly::Unit> p;
+  auto sf = p.getSemiFuture();
+  EXPECT_THROW(std::move(sf).result(), FutureNotReady);
+}
+
 TEST(SemiFuture, SimpleDefer) {
   std::atomic<int> innerResult{0};
   Promise<folly::Unit> p;