From 2a96070177d4aa7e5a9a551ebb8fcdc192d8a195 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Mon, 10 Feb 2020 13:53:55 -0500 Subject: [PATCH] refactor: copy PaginationRange from spanner (#168) --- google/cloud/CMakeLists.txt | 6 +- google/cloud/google_cloud_cpp_grpc_utils.bzl | 1 + ...google_cloud_cpp_grpc_utils_unit_tests.bzl | 1 + google/cloud/internal/pagination_range.h | 214 ++++++++++++++++++ .../cloud/internal/pagination_range_test.cc | 185 +++++++++++++++ 5 files changed, 405 insertions(+), 2 deletions(-) create mode 100644 google/cloud/internal/pagination_range.h create mode 100644 google/cloud/internal/pagination_range_test.cc diff --git a/google/cloud/CMakeLists.txt b/google/cloud/CMakeLists.txt index c099b81..223cd83 100644 --- a/google/cloud/CMakeLists.txt +++ b/google/cloud/CMakeLists.txt @@ -331,7 +331,8 @@ if (GOOGLE_CLOUD_CPP_ENABLE_GRPC_UTILS) internal/background_threads_impl.cc internal/background_threads_impl.h internal/completion_queue_impl.cc - internal/completion_queue_impl.h) + internal/completion_queue_impl.h + internal/pagination_range.h) target_link_libraries( google_cloud_cpp_grpc_utils PUBLIC googleapis-c++::rpc_status_protos google_cloud_cpp_common @@ -360,7 +361,8 @@ if (GOOGLE_CLOUD_CPP_ENABLE_GRPC_UTILS) completion_queue_test.cc connection_options_test.cc grpc_error_delegate_test.cc - internal/background_threads_impl_test.cc) + internal/background_threads_impl_test.cc + internal/pagination_range_test.cc) # Export the list of unit tests so the Bazel BUILD file can pick it up. export_list_to_bazel("google_cloud_cpp_grpc_utils_unit_tests.bzl" diff --git a/google/cloud/google_cloud_cpp_grpc_utils.bzl b/google/cloud/google_cloud_cpp_grpc_utils.bzl index 81d0130..00f6d40 100644 --- a/google/cloud/google_cloud_cpp_grpc_utils.bzl +++ b/google/cloud/google_cloud_cpp_grpc_utils.bzl @@ -29,6 +29,7 @@ google_cloud_cpp_grpc_utils_hdrs = [ "internal/async_read_stream_impl.h", "internal/background_threads_impl.h", "internal/completion_queue_impl.h", + "internal/pagination_range.h", ] google_cloud_cpp_grpc_utils_srcs = [ diff --git a/google/cloud/google_cloud_cpp_grpc_utils_unit_tests.bzl b/google/cloud/google_cloud_cpp_grpc_utils_unit_tests.bzl index f4901a9..a8480f1 100644 --- a/google/cloud/google_cloud_cpp_grpc_utils_unit_tests.bzl +++ b/google/cloud/google_cloud_cpp_grpc_utils_unit_tests.bzl @@ -21,4 +21,5 @@ google_cloud_cpp_grpc_utils_unit_tests = [ "connection_options_test.cc", "grpc_error_delegate_test.cc", "internal/background_threads_impl_test.cc", + "internal/pagination_range_test.cc", ] diff --git a/google/cloud/internal/pagination_range.h b/google/cloud/internal/pagination_range.h new file mode 100644 index 0000000..d3a792c --- /dev/null +++ b/google/cloud/internal/pagination_range.h @@ -0,0 +1,214 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_PAGINATION_RANGE_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_PAGINATION_RANGE_H + +#include "google/cloud/status_or.h" +#include "google/cloud/version.h" +#include +#include +#include +#include +#include +#include + +namespace google { +namespace cloud { +inline namespace GOOGLE_CLOUD_CPP_NS { +namespace internal { + +/** + * An input iterator for a class with the same interface as `PaginationRange`. + */ +template +class PaginationIterator { + public: + //@{ + /// @name Iterator traits + using iterator_category = std::input_iterator_tag; + using value_type = StatusOr; + using difference_type = std::ptrdiff_t; + using pointer = value_type*; + using reference = value_type&; + //@} + + PaginationIterator() : owner_(nullptr) {} + + PaginationIterator& operator++() { + *this = owner_->GetNext(); + return *this; + } + + PaginationIterator operator++(int) { + PaginationIterator tmp(*this); + operator++(); + return tmp; + } + + value_type const* operator->() const { return &value_; } + value_type* operator->() { return &value_; } + + value_type const& operator*() const& { return value_; } + value_type& operator*() & { return value_; } +#if GOOGLE_CLOUD_CPP_HAVE_CONST_REF_REF + value_type const&& operator*() const&& { return std::move(value_); } +#endif // GOOGLE_CLOUD_CPP_HAVE_CONST_REF_REF + value_type&& operator*() && { return std::move(value_); } + + private: + friend Range; + + friend bool operator==(PaginationIterator const& lhs, + PaginationIterator const& rhs) { + // Iterators on different streams are always different. + if (lhs.owner_ != rhs.owner_) { + return false; + } + // All end iterators are equal. + if (lhs.owner_ == nullptr) { + return true; + } + // Iterators on the same stream are equal if they point to the same object. + if (lhs.value_.ok() && rhs.value_.ok()) { + return google::protobuf::util::MessageDifferencer::Equals(*lhs.value_, + *rhs.value_); + } + // If one is an error and the other is not then they must be different, + // because only one iterator per range can have an error status. For the + // same reason, if both have an error they both are pointing to the same + // element. + return lhs.value_.ok() == rhs.value_.ok(); + } + + friend bool operator!=(PaginationIterator const& lhs, + PaginationIterator const& rhs) { + return !(lhs == rhs); + } + + PaginationIterator(Range* owner, value_type value) + : owner_(owner), value_(std::move(value)) {} + + Range* owner_; + value_type value_; +}; + +/** + * Adapt pagination APIs to look like input ranges. + * + * A number of gRPC APIs iterate over the elements in a "collection" using + * pagination APIs. The application calls a `List*()` RPC which returns + * a "page" of elements and a token, calling the same `List*()` RPC with the + * token returns the next "page". We want to expose these APIs as input ranges + * in the C++ client libraries. This class performs that work. + * + * @tparam T the type of the items, typically a proto describing the resources + * @tparam Request the type of the request object for the `List` RPC. + * @tparam Response the type of the response object for the `List` RPC. + */ +template +class PaginationRange { + public: + /** + * Create a new range to paginate over some elements. + * + * @param request the first request to start the iteration, the library may + * initialize this request with any filtering constraints. + * @param loader makes the RPC request to fetch a new page of items. + * @param get_items extracts the items from the response using native C++ + * types (as opposed to the proto types used in `Response`). + */ + PaginationRange(Request request, + std::function(Request const& r)> loader, + std::function(Response r)> get_items) + : request_(std::move(request)), + next_page_loader_(std::move(loader)), + get_items_(std::move(get_items)), + on_last_page_(false) { + current_ = current_page_.begin(); + } + + /// The iterator type for this Range. + using iterator = PaginationIterator; + + /** + * Return an iterator over the range of `T` objects. + * + * The returned iterator is a single-pass input iterator that reads new `T` + * objects from the underlying `PaginationRange` when incremented. + * + * Creating, and particularly incrementing, multiple iterators on the same + * PaginationRange<> is unsupported and can produce incorrect results. + */ + iterator begin() { return GetNext(); } + + /// Return an iterator pointing to the end of the stream. + iterator end() { return PaginationIterator{}; } + + protected: + friend class PaginationIterator; + + /** + * Fetches (or returns if already fetched) the next object from the stream. + * + * @return An iterator pointing to the next element in the stream. On error, + * it returns an iterator that is different from `.end()`, but has an error + * status. If the stream is exhausted, it returns the `.end()` iterator. + */ + iterator GetNext() { + static Status const kPastTheEndError( + StatusCode::kFailedPrecondition, + "Cannot iterating past the end of ListObjectReader"); + if (current_page_.end() == current_) { + if (on_last_page_) { + return iterator(nullptr, kPastTheEndError); + } + request_.set_page_token(std::move(next_page_token_)); + auto response = next_page_loader_(request_); + if (!response.ok()) { + next_page_token_.clear(); + current_page_.clear(); + on_last_page_ = true; + current_ = current_page_.begin(); + return iterator(this, std::move(response).status()); + } + next_page_token_ = std::move(*response->mutable_next_page_token()); + current_page_ = get_items_(*std::move(response)); + current_ = current_page_.begin(); + if (next_page_token_.empty()) { + on_last_page_ = true; + } + if (current_page_.end() == current_) { + return iterator(nullptr, kPastTheEndError); + } + } + return iterator(this, std::move(*current_++)); + } + + private: + Request request_; + std::function(Request const& r)> next_page_loader_; + std::function(Response r)> get_items_; + std::vector current_page_; + typename std::vector::iterator current_; + std::string next_page_token_; + bool on_last_page_; +}; + +} // namespace internal +} // namespace GOOGLE_CLOUD_CPP_NS +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_PAGINATION_RANGE_H diff --git a/google/cloud/internal/pagination_range_test.cc b/google/cloud/internal/pagination_range_test.cc new file mode 100644 index 0000000..fe90e37 --- /dev/null +++ b/google/cloud/internal/pagination_range_test.cc @@ -0,0 +1,185 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "google/cloud/internal/pagination_range.h" +#include +#include + +namespace google { +namespace cloud { +inline namespace GOOGLE_CLOUD_CPP_NS { +namespace internal { +namespace { + +using ::testing::_; +using ::testing::ElementsAre; +using ::testing::HasSubstr; +using ::testing::Invoke; +using ItemType = ::google::bigtable::admin::v2::AppProfile; +using Request = ::google::bigtable::admin::v2::ListAppProfilesRequest; +using Response = ::google::bigtable::admin::v2::ListAppProfilesResponse; +using TestedRange = PaginationRange; + +class MockRpc { + public: + MOCK_METHOD1(Loader, StatusOr(Request const&)); + MOCK_METHOD1(GetItems, std::vector(Response)); +}; + +std::vector GetItems(Response const& response) { + return std::vector(response.app_profiles().begin(), + response.app_profiles().end()); +} + +TEST(RangeFromPagination, Empty) { + MockRpc mock; + EXPECT_CALL(mock, Loader(_)).WillOnce(Invoke([](Request const& request) { + EXPECT_TRUE(request.page_token().empty()); + Response response; + response.clear_next_page_token(); + return response; + })); + + TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, + GetItems); + EXPECT_TRUE(range.begin() == range.end()); +} + +TEST(RangeFromPagination, SinglePage) { + MockRpc mock; + EXPECT_CALL(mock, Loader(_)).WillOnce(Invoke([](Request const& request) { + EXPECT_TRUE(request.page_token().empty()); + Response response; + response.clear_next_page_token(); + response.add_app_profiles()->set_name("p1"); + response.add_app_profiles()->set_name("p2"); + return response; + })); + + TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, + GetItems); + std::vector names; + for (auto& p : range) { + if (!p) break; + names.push_back(p->name()); + } + EXPECT_THAT(names, ElementsAre("p1", "p2")); +} + +TEST(RangeFromPagination, TwoPages) { + MockRpc mock; + EXPECT_CALL(mock, Loader(_)) + .WillOnce(Invoke([](Request const& request) { + EXPECT_TRUE(request.page_token().empty()); + Response response; + response.set_next_page_token("t1"); + response.add_app_profiles()->set_name("p1"); + response.add_app_profiles()->set_name("p2"); + return response; + })) + .WillOnce(Invoke([](Request const& request) { + EXPECT_EQ("t1", request.page_token()); + Response response; + response.clear_next_page_token(); + response.add_app_profiles()->set_name("p3"); + response.add_app_profiles()->set_name("p4"); + return response; + })); + + TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, + GetItems); + std::vector names; + for (auto& p : range) { + if (!p) break; + names.push_back(p->name()); + } + EXPECT_THAT(names, ElementsAre("p1", "p2", "p3", "p4")); +} + +TEST(RangeFromPagination, TwoPagesWithError) { + MockRpc mock; + EXPECT_CALL(mock, Loader(_)) + .WillOnce(Invoke([](Request const& request) { + EXPECT_TRUE(request.page_token().empty()); + Response response; + response.set_next_page_token("t1"); + response.add_app_profiles()->set_name("p1"); + response.add_app_profiles()->set_name("p2"); + return response; + })) + .WillOnce(Invoke([](Request const& request) { + EXPECT_EQ("t1", request.page_token()); + Response response; + response.set_next_page_token("t2"); + response.add_app_profiles()->set_name("p3"); + response.add_app_profiles()->set_name("p4"); + return response; + })) + .WillOnce(Invoke([](Request const& request) { + EXPECT_EQ("t2", request.page_token()); + return Status(StatusCode::kAborted, "bad-luck"); + })); + + TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, + GetItems); + std::vector names; + for (auto& p : range) { + if (!p) { + EXPECT_EQ(StatusCode::kAborted, p.status().code()); + EXPECT_THAT(p.status().message(), HasSubstr("bad-luck")); + break; + } + names.push_back(p->name()); + } + EXPECT_THAT(names, ElementsAre("p1", "p2", "p3", "p4")); +} + +TEST(RangeFromPagination, IteratorCoverage) { + MockRpc mock; + EXPECT_CALL(mock, Loader(_)) + .WillOnce(Invoke([](Request const& request) { + EXPECT_TRUE(request.page_token().empty()); + Response response; + response.set_next_page_token("t1"); + response.add_app_profiles()->set_name("p1"); + return response; + })) + .WillOnce(Invoke([](Request const& request) { + EXPECT_EQ("t1", request.page_token()); + return Status(StatusCode::kAborted, "bad-luck"); + })); + + TestedRange range(Request{}, [&](Request const& r) { return mock.Loader(r); }, + GetItems); + auto i0 = range.begin(); + auto i1 = i0; + EXPECT_TRUE(i0 == i1); + EXPECT_FALSE(i1 == range.end()); + ++i1; + auto i2 = i1; + EXPECT_FALSE(i0 == i1); + EXPECT_TRUE(i1 == i2); + ASSERT_FALSE(i1 == range.end()); + auto& item = *i1; + EXPECT_EQ(StatusCode::kAborted, item.status().code()); + EXPECT_THAT(item.status().message(), HasSubstr("bad-luck")); + ++i1; + EXPECT_TRUE(i1 == range.end()); +} + +} // namespace +} // namespace internal +} // namespace GOOGLE_CLOUD_CPP_NS +} // namespace cloud +} // namespace google