From 214acc9bbf4c8f7b76bb921c74d0df1bcda2d687 Mon Sep 17 00:00:00 2001
From: Danfeng Wang <sunniewang@meta.com>
Date: Mon, 3 Feb 2025 11:22:59 -0800
Subject: [PATCH] Maps keys, items, and values return views, not generators

Summary:
This diff is implementing `MapViews` for `py3` and fixing the failed unit test.

[Context Post](https://fb.workplace.com/groups/1730279463893632/permalink/3935447050043518/): Thrift-py3 `.values()`, `.items()`, `.keys()` return generator instead of a python view. This causes incompatibility issues for users upgrading to thrift-python. It also means some view apis like `len` don't work, and mean the return object is only once-iterable.

User Request: https://fb.workplace.com/groups/thrift.py3/permalink/1480673825975358/

#buildall

Reviewed By: yoney, ahilger

Differential Revision: D68674342

fbshipit-source-id: 09dcb6f79d04ed70b1eb70bb0b6898a70ed35105
---
 .../src/thrift/lib/py3/test/auto_migrate/maps.py    |  7 +++++++
 third-party/thrift/src/thrift/lib/py3/types.pyx     | 13 +++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/third-party/thrift/src/thrift/lib/py3/test/auto_migrate/maps.py b/third-party/thrift/src/thrift/lib/py3/test/auto_migrate/maps.py
index c4da262f8f8a83..6298e78ee53cdb 100644
--- a/third-party/thrift/src/thrift/lib/py3/test/auto_migrate/maps.py
+++ b/third-party/thrift/src/thrift/lib/py3/test/auto_migrate/maps.py
@@ -79,6 +79,13 @@ def test_isinstance(self) -> None:
         self.assertNotIsInstance(StrIntMap(str_int_map), Map__Color_Color)
         self.assertNotIsInstance(StrIntMap(str_int_map), Map__string_List__i32)
 
+    def test_map_views(self) -> None:
+        str_int_map = StrIntMap({"foo": 5})
+        self.assertEqual(len(str_int_map), 1)
+        self.assertEqual(len(str_int_map.keys()), 1)
+        self.assertEqual(len(str_int_map.values()), 1)
+        self.assertEqual(len(str_int_map.items()), 1)
+
     def test_getitem(self) -> None:
         x = StrStrMap({"test": "value"})
         self.assertEqual(x["test"], "value")
diff --git a/third-party/thrift/src/thrift/lib/py3/types.pyx b/third-party/thrift/src/thrift/lib/py3/types.pyx
index 54e588d970f324..7949957afad506 100644
--- a/third-party/thrift/src/thrift/lib/py3/types.pyx
+++ b/third-party/thrift/src/thrift/lib/py3/types.pyx
@@ -672,19 +672,12 @@ cdef class Map(Container):
         yield from self._py_obj
 
     def keys(Map self):
-        return self.__iter__()
+        return self._py_obj.keys()
 
     def values(Map self):
-        if not self:
-            return
-        yield from self._py_obj.values()
-
+        return self._py_obj.values()
     def items(Map self):
-        if not self:
-            return
-        yield from self._py_obj.items()
-
-
+        return self._py_obj.items()
 CompiledEnum = _fbthrift_python_Enum
 Enum = _fbthrift_python_Enum
 # I wanted to call the base class Enum, but there is a cython bug