Fix bug in the from_pb() functions of the *Filter classes (#20)

The `*Filter` Classes can take in `None` parameters (when we don't wanna
filter for those). This was however not well handled in the `from_pb()`
functions. This PR takes care of fixing this and adds better test
coverage for those cases.

RELEASE_NOTES.md

@@ -7,7 +7,8 @@
 ## Upgrading
 
 - Refactor order states following `frequenz-api-electricity-trading` update to <branch>:
-  * Remove obsolete order states CANCEL_REQUESTED and CANCEL_REJECTED
+
+  - Remove obsolete order states CANCEL_REQUESTED and CANCEL_REJECTED
 
 - The minimum required version of `frequenz-client-base` is now 0.3
 
@@ -19,4 +20,4 @@
 
 ## Bug Fixes
 
-<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
+- Handle missing protobuf fields in Filter classes

src/frequenz/client/electricity_trading/_types.py

@@ -1399,15 +1399,31 @@ def from_pb(
             GridpoolOrderFilter object corresponding to the protobuf message.
         """
         return cls(
-            order_states=[
-                OrderState.from_pb(state) for state in gridpool_order_filter.states
-            ],
-            side=MarketSide.from_pb(gridpool_order_filter.side),
-            delivery_period=DeliveryPeriod.from_pb(
-                gridpool_order_filter.delivery_period
+            order_states=(
+                [OrderState.from_pb(state) for state in gridpool_order_filter.states]
+                if gridpool_order_filter.states
+                else None
+            ),
+            side=(
+                MarketSide.from_pb(gridpool_order_filter.side)
+                if gridpool_order_filter.HasField("side")
+                else None
+            ),
+            delivery_period=(
+                DeliveryPeriod.from_pb(gridpool_order_filter.delivery_period)
+                if gridpool_order_filter.HasField("delivery_period")
+                else None
+            ),
+            delivery_area=(
+                DeliveryArea.from_pb(gridpool_order_filter.delivery_area)
+                if gridpool_order_filter.HasField("delivery_area")
+                else None
+            ),
+            tag=(
+                gridpool_order_filter.tag
+                if gridpool_order_filter.HasField("tag")
+                else None
             ),
-            delivery_area=DeliveryArea.from_pb(gridpool_order_filter.delivery_area),
-            tag=gridpool_order_filter.tag,
         )
 
     def to_pb(self) -> electricity_trading_pb2.GridpoolOrderFilter:
@@ -1507,15 +1523,31 @@ def from_pb(
             GridpoolTradeFilter object corresponding to the protobuf message.
         """
         return cls(
-            trade_states=[
-                TradeState.from_pb(state) for state in gridpool_trade_filter.states
-            ],
-            trade_ids=list(gridpool_trade_filter.trade_ids),
-            side=MarketSide.from_pb(gridpool_trade_filter.side),
-            delivery_period=DeliveryPeriod.from_pb(
-                gridpool_trade_filter.delivery_period
+            trade_states=(
+                [TradeState.from_pb(state) for state in gridpool_trade_filter.states]
+                if gridpool_trade_filter.states
+                else None
+            ),
+            trade_ids=(
+                list(gridpool_trade_filter.trade_ids)
+                if gridpool_trade_filter.trade_ids
+                else None
+            ),
+            side=(
+                MarketSide.from_pb(gridpool_trade_filter.side)
+                if gridpool_trade_filter.HasField("side")
+                else None
+            ),
+            delivery_period=(
+                DeliveryPeriod.from_pb(gridpool_trade_filter.delivery_period)
+                if gridpool_trade_filter.HasField("delivery_period")
+                else None
+            ),
+            delivery_area=(
+                DeliveryArea.from_pb(gridpool_trade_filter.delivery_area)
+                if gridpool_trade_filter.HasField("delivery_area")
+                else None
             ),
-            delivery_area=DeliveryArea.from_pb(gridpool_trade_filter.delivery_area),
         )
 
     def to_pb(self) -> electricity_trading_pb2.GridpoolTradeFilter:
@@ -1604,13 +1636,25 @@ def from_pb(
             PublicTradeFilter object corresponding to the protobuf message.
         """
         return cls(
-            states=[TradeState.from_pb(state) for state in public_trade_filter.states],
-            delivery_period=DeliveryPeriod.from_pb(public_trade_filter.delivery_period),
-            buy_delivery_area=DeliveryArea.from_pb(
-                public_trade_filter.buy_delivery_area
+            states=(
+                [TradeState.from_pb(state) for state in public_trade_filter.states]
+                if public_trade_filter.states
+                else None
             ),
-            sell_delivery_area=DeliveryArea.from_pb(
-                public_trade_filter.sell_delivery_area
+            delivery_period=(
+                DeliveryPeriod.from_pb(public_trade_filter.delivery_period)
+                if public_trade_filter.HasField("delivery_period")
+                else None
+            ),
+            buy_delivery_area=(
+                DeliveryArea.from_pb(public_trade_filter.buy_delivery_area)
+                if public_trade_filter.HasField("buy_delivery_area")
+                else None
+            ),
+            sell_delivery_area=(
+                DeliveryArea.from_pb(public_trade_filter.sell_delivery_area)
+                if public_trade_filter.HasField("sell_delivery_area")
+                else None
             ),
         )
 

tests/test_types.py

@@ -33,6 +33,7 @@
     OrderType,
     Price,
     PublicTrade,
+    PublicTradeFilter,
     StateDetail,
     StateReason,
     Trade,
@@ -196,6 +197,32 @@
     tag="test",
 )
 
+GRIDPOOL_ORDER_FILTER_EMPTY = GridpoolOrderFilter()
+GRIDPOOL_ORDER_FILTER_EMPTY_PB = electricity_trading_pb2.GridpoolOrderFilter()
+
+PUBLIC_TRADE_FILTER = PublicTradeFilter(
+    states=[TradeState.ACTIVE, TradeState.CANCELED],
+    buy_delivery_area=DeliveryArea(
+        code="XYZ", code_type=EnergyMarketCodeType.EUROPE_EIC
+    ),
+    sell_delivery_area=None,
+    delivery_period=DeliveryPeriod(start=START_TIME, duration=timedelta(minutes=15)),
+)
+PUBLIC_TRADE_FILTER_PB = electricity_trading_pb2.PublicTradeFilter(
+    states=[
+        electricity_trading_pb2.TradeState.TRADE_STATE_ACTIVE,
+        electricity_trading_pb2.TradeState.TRADE_STATE_CANCELED,
+    ],
+    buy_delivery_area=delivery_area_pb2.DeliveryArea(
+        code="XYZ",
+        code_type=delivery_area_pb2.EnergyMarketCodeType.ENERGY_MARKET_CODE_TYPE_EUROPE_EIC,
+    ),
+    delivery_period=delivery_duration_pb2.DeliveryPeriod(
+        start=START_TIME_PB,
+        duration=delivery_duration_pb2.DeliveryDuration.DELIVERY_DURATION_15,
+    ),
+)
+
 UPDATE_ORDER = UpdateOrder(price=Price(amount=Decimal("100.00"), currency=Currency.USD))
 UPDATE_ORDER_PB = electricity_trading_pb2.UpdateGridpoolOrderRequest.UpdateOrder(
     price=price_pb2.Price(
@@ -487,7 +514,7 @@ def test_order_detail_timezone_converted_to_utc() -> None:
 
 
 def test_gridpool_order_filter_to_pb() -> None:
-    """Test the client gridpool type conversion to protobuf."""
+    """Test the client gridpool order filter type conversion to protobuf."""
     assert_conversion_to_pb(
         original=GRIDPOOL_ORDER_FILTER,
         expected_pb=GRIDPOOL_ORDER_FILTER_PB,
@@ -496,14 +523,50 @@ def test_gridpool_order_filter_to_pb() -> None:
 
 
 def test_gridpool_order_filter_from_pb() -> None:
-    """Test the client gridpool type conversion from protobuf."""
+    """Test the client gridpool order filter type conversion from protobuf."""
     assert_conversion_from_pb(
         original_pb=GRIDPOOL_ORDER_FILTER_PB,
         expected=GRIDPOOL_ORDER_FILTER,
         assert_func=assert_equal,
     )
 
 
+def test_gridpool_order_filter_empty_to_pb() -> None:
+    """Test the client empty gridpool order filter type conversion to protobuf."""
+    assert_conversion_to_pb(
+        original=GRIDPOOL_ORDER_FILTER_EMPTY,
+        expected_pb=GRIDPOOL_ORDER_FILTER_EMPTY_PB,
+        assert_func=assert_equal,
+    )
+
+
+def test_gridpool_order_filter_empty_from_pb() -> None:
+    """Test the client empty gridpool order filter type conversion from protobuf."""
+    assert_conversion_from_pb(
+        original_pb=GRIDPOOL_ORDER_FILTER_EMPTY_PB,
+        expected=GRIDPOOL_ORDER_FILTER_EMPTY,
+        assert_func=assert_equal,
+    )
+
+
+def test_public_trade_filter_to_pb() -> None:
+    """Test the client public trade filter type conversion to protobuf."""
+    assert_conversion_to_pb(
+        original=PUBLIC_TRADE_FILTER,
+        expected_pb=PUBLIC_TRADE_FILTER_PB,
+        assert_func=assert_equal,
+    )
+
+
+def test_public_trade_filter_from_pb() -> None:
+    """Test the client public trade filter type conversion from protobuf."""
+    assert_conversion_from_pb(
+        original_pb=PUBLIC_TRADE_FILTER_PB,
+        expected=PUBLIC_TRADE_FILTER,
+        assert_func=assert_equal,
+    )
+
+
 def test_update_order_to_pb() -> None:
     """Test the client update order type conversion to protobuf."""
     converted_update_order = UPDATE_ORDER.to_pb()