-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Paginator template and unit test #3681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| /** | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0. | ||
| */ | ||
|
|
||
| #pragma once | ||
|
|
||
| namespace Aws | ||
| { | ||
| namespace Utils | ||
| { | ||
| namespace Pagination | ||
| { | ||
|
|
||
| template <class ServiceClient, class OperationRequest, class OperationTraits> | ||
| class PagePaginator | ||
| { | ||
| public: | ||
| using OutcomeType = typename OperationTraits::OutcomeType; | ||
| using ResultType = typename OperationTraits::ResultType; | ||
|
|
||
| class PageIterator | ||
| { | ||
| public: | ||
| PageIterator() = default; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why have a default constructor? deafult initialization doesnt sound valid |
||
|
|
||
| PageIterator(ServiceClient* client, const OperationRequest& firstReq) | ||
| : m_client(client), | ||
| m_request(firstReq), | ||
| m_atEnd(false) | ||
| { | ||
| FetchPage(); | ||
| } | ||
|
|
||
| const OutcomeType& operator*() const { return m_currentOutcome; } | ||
|
|
||
| PageIterator& operator++() | ||
| { | ||
| if (m_atEnd) return *this; | ||
|
|
||
| // If last fetch failed, end iteration | ||
| if (!m_currentOutcome.IsSuccess()) | ||
| { | ||
| m_atEnd = true; | ||
| return *this; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how will the error in this case be shown to the customer? your test only works because the initial request fails in the constructor, what if the second or subsequent requests fail? lets add a test for this and fix this. |
||
| } | ||
|
|
||
| // If no more results, end iteration | ||
| if (!OperationTraits::HasMoreResults(m_currentOutcome.GetResult())) | ||
| { | ||
| m_atEnd = true; | ||
| return *this; | ||
| } | ||
|
|
||
| // Mutate iterator-owned request for next page | ||
| OperationTraits::SetNextRequest(m_currentOutcome.GetResult(), m_request); | ||
| FetchPage(); | ||
| return *this; | ||
| } | ||
|
|
||
| bool operator!=(const PageIterator& other) const | ||
| { | ||
| return !(m_atEnd && other.m_atEnd); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why for not equals are you only comparing the boolean |
||
| } | ||
|
|
||
| bool operator==(const PageIterator& other) const | ||
| { | ||
| return !(*this != other); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets flip this so that equals actually compares and not equals is the negation of equals |
||
| } | ||
|
|
||
| private: | ||
| void FetchPage() | ||
| { | ||
| m_currentOutcome = OperationTraits::Invoke(*m_client, m_request); | ||
| } | ||
|
|
||
| ServiceClient* m_client{}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if its a raw pointer who owns it? who deletes it? |
||
| OperationRequest m_request{}; | ||
| OutcomeType m_currentOutcome{}; | ||
| bool m_atEnd{true}; | ||
| }; | ||
|
|
||
| PagePaginator(ServiceClient* client, const OperationRequest& firstReq) | ||
| : m_client(client), | ||
| m_firstRequest(firstReq) {} | ||
|
|
||
| PageIterator begin() const { return PageIterator(m_client, m_firstRequest); } | ||
| PageIterator end() const { return PageIterator(); } | ||
|
|
||
| private: | ||
| ServiceClient* m_client{}; | ||
| OperationRequest m_firstRequest{}; | ||
| }; | ||
|
|
||
| } // namespace Pagination | ||
| } // namespace Utils | ||
| } // namespace Aws | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ file(GLOB AWS_CLIENT_SRC "${CMAKE_CURRENT_SOURCE_DIR}/aws/client/*.cpp") | |
| file(GLOB AWS_NET_SRC "${CMAKE_CURRENT_SOURCE_DIR}/aws/net/*.cpp") | ||
| file(GLOB HTTP_SRC "${CMAKE_CURRENT_SOURCE_DIR}/http/*.cpp") | ||
| file(GLOB UTILS_SRC "${CMAKE_CURRENT_SOURCE_DIR}/utils/*.cpp") | ||
| file(GLOB UTILS_PAGINATION_SRC "${CMAKE_CURRENT_SOURCE_DIR}/utils/pagination/*.cpp") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no |
||
| file(GLOB UTILS_CRYPTO_SRC "${CMAKE_CURRENT_SOURCE_DIR}/utils/crypto/*.cpp") | ||
| file(GLOB UTILS_EVENT_SRC "${CMAKE_CURRENT_SOURCE_DIR}/utils/event/*.cpp") | ||
| file(GLOB UTILS_JSON_SRC "${CMAKE_CURRENT_SOURCE_DIR}/utils/json/*.cpp") | ||
|
|
@@ -41,6 +42,7 @@ file(GLOB AWS_CPP_SDK_CORE_TESTS_SRC | |
| ${AWS_NET_SRC} | ||
| ${HTTP_SRC} | ||
| ${UTILS_SRC} | ||
| ${UTILS_PAGINATION_SRC} | ||
| ${UTILS_CRYPTO_SRC} | ||
| ${UTILS_EVENT_SRC} | ||
| ${UTILS_JSON_SRC} | ||
|
|
@@ -69,6 +71,7 @@ if(PLATFORM_WINDOWS) | |
| source_group("Source Files\\http" FILES ${HTTP_SRC}) | ||
| source_group("Source Files\\monitoring" FILES ${MONITORING_SRC}) | ||
| source_group("Source Files\\utils" FILES ${UTILS_SRC}) | ||
| source_group("Source Files\\utils\\pagination" FILES ${UTILS_PAGINATION_SRC}) | ||
| source_group("Source Files\\utils\\crypto" FILES ${UTILS_CRYPTO_SRC}) | ||
| source_group("Source Files\\utils\\event" FILES ${UTILS_EVENT_SRC}) | ||
| source_group("Source Files\\utils\\json" FILES ${UTILS_JSON_SRC}) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| /** | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0. | ||
| */ | ||
|
|
||
| #include <aws/testing/AwsCppSdkGTestSuite.h> | ||
| #include <aws/core/utils/pagination/Paginator.h> | ||
| #include <aws/core/utils/StringUtils.h> | ||
| #include <vector> | ||
| #include <string> | ||
|
|
||
| using namespace Aws::Utils::Pagination; | ||
| using namespace Aws::Utils; | ||
|
|
||
| struct TestResult { | ||
| Aws::Vector<Aws::String> items; | ||
| Aws::String outputToken; | ||
| bool moreResults; | ||
|
|
||
| const Aws::Vector<Aws::String>& GetItems() const { return items; } | ||
| const Aws::String& GetOutputToken() const { return outputToken; } | ||
| bool GetMoreResults() const { return moreResults; } | ||
| }; | ||
|
|
||
| struct TestOutcome { | ||
| bool success; | ||
| Aws::String error; | ||
| TestResult result; | ||
|
|
||
| bool IsSuccess() const { return success; } | ||
| const Aws::String& GetError() const { return error; } | ||
| const TestResult& GetResult() const { return result; } | ||
| }; | ||
|
|
||
| struct TestRequest { | ||
| Aws::String inputToken; | ||
| int limitKey = 3; | ||
|
|
||
| void SetInputToken(const Aws::String& t) { inputToken = t; } | ||
| const Aws::String& GetInputToken() const { return inputToken; } | ||
| }; | ||
|
|
||
| class PaginatorTestClient { | ||
| private: | ||
| Aws::Vector<Aws::String> allData; | ||
| bool shouldFail = false; | ||
|
|
||
| public: | ||
| PaginatorTestClient() : allData({"apple", "banana", "cherry", "dragon-fruit", "elderberry", "fig", "grape"}) {} | ||
|
|
||
| void SetData(const Aws::Vector<Aws::String>& data) { allData = data; } | ||
| void SetShouldFail(bool fail) { shouldFail = fail; } | ||
| void Reset() { | ||
| allData = {"apple", "banana", "cherry", "dragon-fruit", "elderberry", "fig", "grape"}; | ||
| shouldFail = false; | ||
| } | ||
|
|
||
| TestOutcome ListItems(const TestRequest& request) { | ||
| if (shouldFail) { | ||
| return {false, "Request failed", {}}; | ||
| } | ||
|
|
||
| int startIdx = 0; | ||
| if (!request.GetInputToken().empty()) { | ||
| startIdx = StringUtils::ConvertToInt32(request.GetInputToken().c_str()); | ||
| } | ||
|
|
||
| TestResult result; | ||
| for (size_t i = 0; i < static_cast<size_t>(request.limitKey) && startIdx + i < allData.size(); ++i) { | ||
| result.items.push_back(allData[startIdx + i]); | ||
| } | ||
|
|
||
| int nextIdx = startIdx + request.limitKey; | ||
| if (static_cast<size_t>(nextIdx) < allData.size()) { | ||
| result.outputToken = StringUtils::to_string(nextIdx); | ||
| result.moreResults = true; | ||
| } else { | ||
| result.moreResults = false; | ||
| } | ||
|
|
||
| return {true, "", result}; | ||
| } | ||
| }; | ||
|
|
||
| struct ListItemsTraits { | ||
| using OutcomeType = TestOutcome; | ||
| using ResultType = TestResult; | ||
|
|
||
| static OutcomeType Invoke(PaginatorTestClient& client, const TestRequest& request) { | ||
| return client.ListItems(request); | ||
| } | ||
|
|
||
| static bool HasMoreResults(const TestResult& result) { | ||
| return result.GetMoreResults(); | ||
| } | ||
|
|
||
| static void SetNextRequest(const TestResult& result, TestRequest& request) { | ||
| request.SetInputToken(result.GetOutputToken()); | ||
| } | ||
| }; | ||
|
|
||
| class PaginatorTest : public Aws::Testing::AwsCppSdkGTestSuite | ||
| { | ||
| protected: | ||
| void SetUp() override | ||
| { | ||
| client.Reset(); | ||
| } | ||
|
|
||
| void TearDown() override | ||
| { | ||
| client.Reset(); | ||
| } | ||
|
|
||
| PaginatorTestClient client; | ||
| TestRequest request; | ||
| }; | ||
|
|
||
| TEST_F(PaginatorTest, TestIteratesThroughAllPages) | ||
| { | ||
| PagePaginator<PaginatorTestClient, TestRequest, ListItemsTraits> paginator(&client, request); | ||
|
|
||
| Aws::Vector<Aws::String> allItems; | ||
| int pageCount = 0; | ||
|
|
||
| for (const auto& outcome : paginator) | ||
| { | ||
| ASSERT_TRUE(outcome.IsSuccess()); | ||
| pageCount++; | ||
|
|
||
| const auto& page = outcome.GetResult(); | ||
| for (const auto& item : page.GetItems()) | ||
| { | ||
| allItems.push_back(item); | ||
| } | ||
| } | ||
|
|
||
| EXPECT_EQ(pageCount, 3); // 7 items / 3 per page = 3 pages | ||
| EXPECT_EQ(allItems.size(), 7u); | ||
| EXPECT_STREQ(allItems[0].c_str(), "apple"); | ||
| EXPECT_STREQ(allItems[6].c_str(), "grape"); | ||
| } | ||
|
|
||
| TEST_F(PaginatorTest, TestHandlesErrorGracefully) | ||
| { | ||
| client.SetShouldFail(true); | ||
| PagePaginator<PaginatorTestClient, TestRequest, ListItemsTraits> paginator(&client, request); | ||
|
|
||
| auto it = paginator.begin(); | ||
| ASSERT_TRUE(it != paginator.end()); | ||
|
|
||
| const auto& outcome = *it; | ||
| EXPECT_FALSE(outcome.IsSuccess()); | ||
| EXPECT_STREQ(outcome.GetError().c_str(), "Request failed"); | ||
|
|
||
| ++it; | ||
| EXPECT_TRUE(it == paginator.end()); | ||
| } | ||
|
|
||
| TEST_F(PaginatorTest, TestEmptyResultSet) | ||
| { | ||
| client.SetData({}); | ||
| PagePaginator<PaginatorTestClient, TestRequest, ListItemsTraits> paginator(&client, request); | ||
|
|
||
| auto it = paginator.begin(); | ||
| ASSERT_TRUE(it != paginator.end()); | ||
|
|
||
| const auto& outcome = *it; | ||
| EXPECT_TRUE(outcome.IsSuccess()); | ||
| EXPECT_TRUE(outcome.GetResult().GetItems().empty()); | ||
| EXPECT_FALSE(outcome.GetResult().GetMoreResults()); | ||
|
|
||
| ++it; | ||
| EXPECT_TRUE(it == paginator.end()); | ||
| } | ||
|
|
||
| TEST_F(PaginatorTest, TestBeginEndIteratorComparison) | ||
| { | ||
| PagePaginator<PaginatorTestClient, TestRequest, ListItemsTraits> paginator(&client, request); | ||
|
|
||
| auto begin = paginator.begin(); | ||
| auto end = paginator.end(); | ||
|
|
||
| EXPECT_TRUE(begin != end); | ||
| EXPECT_FALSE(begin == end); | ||
|
|
||
| while (begin != end) | ||
| { | ||
| ++begin; | ||
| } | ||
|
|
||
| EXPECT_TRUE(begin == end); | ||
| EXPECT_FALSE(begin != end); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
think we want to make this a
iterator_traityeah? i.e.i.e.
this is so that std lib algorithms and other iterator based libraries can treat it as a iterator officially