From 2739924c7f318b6c803c1dec87dbf6ab01ad204f Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Mon, 18 Aug 2025 19:02:49 +0200 Subject: [PATCH 1/2] refactor(pagination): Deprecate `Params` and `Info` and add `PaginationInfo` Signed-off-by: Mathias L. Baumann --- RELEASE_NOTES.md | 5 +- .../client/common/pagination/__init__.py | 81 +++++++++++++++++-- tests/test_pagination.py | 64 +++++++++++++++ 3 files changed, 141 insertions(+), 9 deletions(-) create mode 100644 tests/test_pagination.py diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 72d8090..e9042bb 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -5,13 +5,16 @@ ## Upgrading - +- The `pagination.Params` class is deprecated; use the protobuf message directly. +- The `pagination.Info` class is deprecated in favor of the new `pagination.PaginationInfo` class. ## New Features - Mapping for the new `Event` message has been added. - Add new common API enums for `ElectricalComponent` (previously `Components`). +- Added `v1alpha8` variants of the pagination data structures. + ## Bug Fixes - Updated display of protobuf version warnings diff --git a/src/frequenz/client/common/pagination/__init__.py b/src/frequenz/client/common/pagination/__init__.py index 77c91bb..4f3c22f 100644 --- a/src/frequenz/client/common/pagination/__init__.py +++ b/src/frequenz/client/common/pagination/__init__.py @@ -9,12 +9,25 @@ from typing import Self # pylint: disable=no-name-in-module -from frequenz.api.common.v1.pagination.pagination_info_pb2 import PaginationInfo -from frequenz.api.common.v1.pagination.pagination_params_pb2 import PaginationParams +from frequenz.api.common.v1.pagination.pagination_info_pb2 import ( + PaginationInfo as PBPaginationInfo, +) +from frequenz.api.common.v1.pagination.pagination_params_pb2 import ( + PaginationParams as PBPaginationParams, +) +from frequenz.api.common.v1alpha8.pagination.pagination_info_pb2 import ( + PaginationInfo as PBPaginationInfoAlpha8, +) +from typing_extensions import deprecated # pylint: enable=no-name-in-module +@deprecated( + "Params is deprecated, use " + "frequenz.api.common.v1.pagination.pagination_params_pb2.PaginationParams" + " from the API directly instead.", +) @dataclass(frozen=True, kw_only=True) class Params: """Parameters for paginating list requests.""" @@ -26,7 +39,7 @@ class Params: """The token identifying a specific page of the list results.""" @classmethod - def from_proto(cls, pagination_params: PaginationParams) -> Self: + def from_proto(cls, pagination_params: PBPaginationParams) -> Self: """Convert a protobuf Params to PaginationParams object. Args: @@ -39,18 +52,21 @@ def from_proto(cls, pagination_params: PaginationParams) -> Self: page_token=pagination_params.page_token, ) - def to_proto(self) -> PaginationParams: + def to_proto(self) -> PBPaginationParams: """Convert a Params object to protobuf PaginationParams. Returns: Protobuf message corresponding to the Params object. """ - return PaginationParams( + return PBPaginationParams( page_size=self.page_size, page_token=self.page_token, ) +@deprecated( + "Info is deprecated, use PaginationInfo instead.", +) @dataclass(frozen=True, kw_only=True) class Info: """Information about the pagination of a list request.""" @@ -62,7 +78,7 @@ class Info: """The token identifying the next page of results.""" @classmethod - def from_proto(cls, pagination_info: PaginationInfo) -> Self: + def from_proto(cls, pagination_info: PBPaginationInfo) -> Self: """Convert a protobuf PBPaginationInfo to Info object. Args: @@ -75,13 +91,62 @@ def from_proto(cls, pagination_info: PaginationInfo) -> Self: next_page_token=pagination_info.next_page_token, ) - def to_proto(self) -> PaginationInfo: + def to_proto(self) -> PBPaginationInfo: """Convert a Info object to protobuf PBPaginationInfo. Returns: Protobuf message corresponding to the Info object. """ - return PaginationInfo( + return PBPaginationInfo( + total_items=self.total_items, + next_page_token=self.next_page_token, + ) + + +@dataclass(frozen=True, kw_only=True) +class PaginationInfo: + """Information about the pagination of a list request.""" + + total_items: int + """The total number of items that match the request.""" + + next_page_token: str | None = None + """The token identifying the next page of results.""" + + @classmethod + def from_proto( + cls, pagination_info: PBPaginationInfoAlpha8 | PBPaginationInfo + ) -> Self: + """Convert a protobuf PBPaginationInfo to Info object. + + Args: + pagination_info: Info to convert. + Returns: + Info object corresponding to the protobuf message. + """ + return cls( + total_items=pagination_info.total_items, + next_page_token=pagination_info.next_page_token, + ) + + def to_proto_v1alpha8(self) -> PBPaginationInfoAlpha8: + """Convert a Info object to protobuf PBPaginationInfo. + + Returns: + Protobuf message corresponding to the Info object. + """ + return PBPaginationInfoAlpha8( + total_items=self.total_items, + next_page_token=self.next_page_token, + ) + + def to_proto(self) -> PBPaginationInfo: + """Convert a Info object to protobuf PBPaginationInfo. + + Returns: + Protobuf message corresponding to the Info object. + """ + return PBPaginationInfo( total_items=self.total_items, next_page_token=self.next_page_token, ) diff --git a/tests/test_pagination.py b/tests/test_pagination.py new file mode 100644 index 0000000..25508b7 --- /dev/null +++ b/tests/test_pagination.py @@ -0,0 +1,64 @@ +# License: MIT +# Copyright © 2024 Frequenz Energy-as-a-Service GmbH + +"""Tests for the pagination module.""" + +import pytest +from frequenz.api.common.v1.pagination.pagination_info_pb2 import ( + PaginationInfo as PBPaginationInfo, +) +from frequenz.api.common.v1alpha8.pagination.pagination_info_pb2 import ( + PaginationInfo as PBPaginationInfoAlpha8, +) + +from frequenz.client.common.pagination import Info, PaginationInfo + + +def test_pagination_info_from_proto_v1() -> None: + """Test the PaginationInfo from_proto method with v1 proto.""" + proto = PBPaginationInfo(total_items=100, next_page_token="token") + info = PaginationInfo.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token == "token" + + +def test_pagination_info_from_proto_v1alpha8() -> None: + """Test the PaginationInfo from_proto method with v1alpha8 proto.""" + proto = PBPaginationInfoAlpha8(total_items=100, next_page_token="token") + info = PaginationInfo.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token == "token" + + +def test_pagination_info_to_proto_v1() -> None: + """Test the PaginationInfo to_proto method.""" + info = PaginationInfo(total_items=100, next_page_token="token") + proto = info.to_proto() + assert proto.total_items == 100 + assert proto.next_page_token == "token" + + +def test_pagination_info_to_proto_v1alpha8() -> None: + """Test the PaginationInfo to_proto_v1alpha8 method.""" + info = PaginationInfo(total_items=100, next_page_token="token") + proto = info.to_proto_v1alpha8() + assert proto.total_items == 100 + assert proto.next_page_token == "token" + + +def test_deprecated_info_from_proto() -> None: + """Test the deprecated Info from_proto method.""" + proto = PBPaginationInfo(total_items=100, next_page_token="token") + with pytest.deprecated_call(): + info = Info.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token == "token" + + +def test_deprecated_info_to_proto() -> None: + """Test the deprecated Info to_proto method.""" + with pytest.deprecated_call(): + info = Info(total_items=100, next_page_token="token") + proto = info.to_proto() + assert proto.total_items == 100 + assert proto.next_page_token == "token" From 5f8c33e98732ddeea90b0bb9eaadb9454e0e71b4 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Tue, 19 Aug 2025 12:24:45 +0200 Subject: [PATCH 2/2] fix(pagination): Handle empty next_page_token Signed-off-by: Mathias L. Baumann --- .../client/common/pagination/__init__.py | 10 +++++- tests/test_pagination.py | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/frequenz/client/common/pagination/__init__.py b/src/frequenz/client/common/pagination/__init__.py index 4f3c22f..54cd5f6 100644 --- a/src/frequenz/client/common/pagination/__init__.py +++ b/src/frequenz/client/common/pagination/__init__.py @@ -124,9 +124,17 @@ def from_proto( Returns: Info object corresponding to the protobuf message. """ + # We check for truthiness here to handle both cases where the token is + # not set (defaults to "") or is explicitly set to "". In both + # situations, we want to return `None`. Using `HasField("next_page_token")` + # would not handle the case where the token is explicitly set to "". return cls( total_items=pagination_info.total_items, - next_page_token=pagination_info.next_page_token, + next_page_token=( + pagination_info.next_page_token + if pagination_info.next_page_token + else None + ), ) def to_proto_v1alpha8(self) -> PBPaginationInfoAlpha8: diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 25508b7..fc050bc 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -30,6 +30,38 @@ def test_pagination_info_from_proto_v1alpha8() -> None: assert info.next_page_token == "token" +def test_pagination_info_from_proto_v1_no_token() -> None: + """Test the PaginationInfo from_proto method with v1 proto and no token.""" + proto = PBPaginationInfo(total_items=100) + info = PaginationInfo.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token is None + + +def test_pagination_info_from_proto_v1alpha8_no_token() -> None: + """Test the PaginationInfo from_proto method with v1alpha8 proto and no token.""" + proto = PBPaginationInfoAlpha8(total_items=100) + info = PaginationInfo.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token is None + + +def test_pagination_info_from_proto_v1_empty_token() -> None: + """Test the PaginationInfo from_proto method with v1 proto and an empty token.""" + proto = PBPaginationInfo(total_items=100, next_page_token="") + info = PaginationInfo.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token is None + + +def test_pagination_info_from_proto_v1alpha8_empty_token() -> None: + """Test the PaginationInfo from_proto method with v1alpha8 proto and an empty token.""" + proto = PBPaginationInfoAlpha8(total_items=100, next_page_token="") + info = PaginationInfo.from_proto(proto) + assert info.total_items == 100 + assert info.next_page_token is None + + def test_pagination_info_to_proto_v1() -> None: """Test the PaginationInfo to_proto method.""" info = PaginationInfo(total_items=100, next_page_token="token")