Enhance FetchXML handling: validate page attribute and update documentation to reflect Record object usage

Author: maksii

src/PowerPlatform/Dataverse/data/_odata.py

@@ -48,7 +48,9 @@
 _GUID_RE = re.compile(r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}")
 _CALL_SCOPE_CORRELATION_ID: ContextVar[Optional[str]] = ContextVar("_CALL_SCOPE_CORRELATION_ID", default=None)
 _DEFAULT_EXPECTED_STATUSES: tuple[int, ...] = (200, 201, 202, 204)
-_MAX_FETCHXML_LENGTH = 16_000  # ~16 KB raw; caps input size to mitigate XML entity expansion attacks; after URL-encoding stays under 32 KB GET URL limit
+_MAX_FETCHXML_LENGTH = (
+    16_000  # ~16 KB raw; caps input size to mitigate XML entity expansion attacks and reduce URL length after encoding
+)
 _MAX_FETCHXML_PAGES = 10_000
 
 
@@ -999,7 +1001,18 @@ def _query_fetchxml(
                     )
             if page_size is not None and root.get("count") is None:
                 root.set("count", str(page_size))
-            if root.get("page") is None:
+            page_attr = root.get("page")
+            if page_attr is not None:
+                try:
+                    page_number = int(page_attr)
+                except ValueError:
+                    page_number = -1
+                if page_number <= 0:
+                    raise ValidationError(
+                        f"FetchXML 'page' attribute must be a positive integer, got '{page_attr}'",
+                        subcode=VALIDATION_FETCHXML_MALFORMED,
+                    )
+            else:
                 root.set("page", "1")
 
         headers = {

src/PowerPlatform/Dataverse/operations/query.py

@@ -90,7 +90,8 @@ def fetchxml(
 
         Executes a FetchXML query against Dataverse via the Web API. The FetchXML
         string must contain a root ``<fetch>`` element with a child ``<entity name='...'>``
-        element. Results are yielded as pages (lists of record dicts).
+        element. Results are yielded as pages (lists of
+        :class:`~PowerPlatform.Dataverse.models.record.Record` objects).
 
         :param fetchxml: Raw FetchXML string (e.g. ``<fetch><entity name='account'>...</entity></fetch>``).
         :type fetchxml: :class:`str`
@@ -102,8 +103,10 @@ def fetchxml(
             If the FetchXML already contains a ``count`` attribute, the ``page_size`` parameter
             is ignored. The ``count`` in FetchXML takes precedence.
 
-        :return: Generator yielding pages, where each page is a list of record dicts.
-        :rtype: :class:`~typing.Iterable` of :class:`list` of :class:`dict`
+        :return: Generator yielding pages, where each page is a list of
+            :class:`~PowerPlatform.Dataverse.models.record.Record` objects.
+        :rtype: :class:`~typing.Iterable` of :class:`list` of
+            :class:`~PowerPlatform.Dataverse.models.record.Record`
 
         :raises ~PowerPlatform.Dataverse.core.errors.ValidationError:
             If ``fetchxml`` is not a string, is empty, or has invalid structure.
@@ -141,6 +144,10 @@ def fetchxml(
 
         def _paged() -> Iterable[List[Record]]:
             with self._client._scoped_odata() as od:
-                yield from od._query_fetchxml(fetchxml, page_size=page_size)
+                entity_name = ""
+                for page in od._query_fetchxml(fetchxml, page_size=page_size):
+                    if not entity_name:
+                        entity_name = od._extract_entity_from_fetchxml(fetchxml)
+                    yield [Record.from_api_response(entity_name, row) for row in page]
 
         return _paged()

tests/unit/data/test_fetchxml.py

@@ -193,7 +193,7 @@ def test_query_fetchxml_multi_page(self):
     def test_query_fetchxml_paging_cookie_decode(self):
         """Verify double URL-decode of paging cookie and injection into fetch element."""
         self._setup_entity_set()
-        from urllib.parse import quote, unquote
+        from urllib.parse import quote
 
         # Cookie value is double-encoded
         inner = "<cookie pagenumber='2' />"
@@ -264,6 +264,31 @@ def test_query_fetchxml_existing_page_preserved(self):
         self.assertIn("page", call_url)
         self.assertIn("3", call_url)
 
+    def test_query_fetchxml_page_non_integer_raises(self):
+        """Raise ValidationError when page attribute is not an integer."""
+        self._setup_entity_set()
+        fetchxml = "<fetch page='abc'><entity name='account'><attribute name='name' /></entity></fetch>"
+        with self.assertRaises(ValidationError) as ctx:
+            list(self.od._query_fetchxml(fetchxml))
+        self.assertEqual(ctx.exception.subcode, VALIDATION_FETCHXML_MALFORMED)
+        self.assertIn("page", str(ctx.exception).lower())
+
+    def test_query_fetchxml_page_zero_raises(self):
+        """Raise ValidationError when page attribute is zero."""
+        self._setup_entity_set()
+        fetchxml = "<fetch page='0'><entity name='account'><attribute name='name' /></entity></fetch>"
+        with self.assertRaises(ValidationError) as ctx:
+            list(self.od._query_fetchxml(fetchxml))
+        self.assertEqual(ctx.exception.subcode, VALIDATION_FETCHXML_MALFORMED)
+
+    def test_query_fetchxml_page_negative_raises(self):
+        """Raise ValidationError when page attribute is negative."""
+        self._setup_entity_set()
+        fetchxml = "<fetch page='-1'><entity name='account'><attribute name='name' /></entity></fetch>"
+        with self.assertRaises(ValidationError) as ctx:
+            list(self.od._query_fetchxml(fetchxml))
+        self.assertEqual(ctx.exception.subcode, VALIDATION_FETCHXML_MALFORMED)
+
     def test_query_fetchxml_morerecords_string_true(self):
         """When morerecords is string 'true' (not boolean), paging continues."""
         self._setup_entity_set()

tests/unit/test_query_operations.py

@@ -58,21 +58,27 @@ def test_sql_empty_result(self):
     # -------------------------------------------------------------- fetchxml
 
     def test_fetchxml_basic(self):
-        """fetchxml() should call _query_fetchxml and return results."""
-        expected_pages = [
+        """fetchxml() should call _query_fetchxml and return Record objects."""
+        raw_pages = [
             [{"accountid": "1", "name": "Contoso"}, {"accountid": "2", "name": "Fabrikam"}],
         ]
-        self.client._odata._query_fetchxml.return_value = iter(expected_pages)
+        self.client._odata._query_fetchxml.return_value = iter(raw_pages)
+        self.client._odata._extract_entity_from_fetchxml.return_value = "account"
 
         fetchxml = "<fetch><entity name='account'><attribute name='name' /></entity></fetch>"
         result = list(self.client.query.fetchxml(fetchxml))
 
         self.client._odata._query_fetchxml.assert_called_once_with(fetchxml, page_size=None)
-        self.assertEqual(result, expected_pages)
+        self.assertEqual(len(result), 1)
+        self.assertEqual(len(result[0]), 2)
+        self.assertIsInstance(result[0][0], Record)
+        self.assertEqual(result[0][0]["name"], "Contoso")
+        self.assertEqual(result[0][1]["name"], "Fabrikam")
 
     def test_fetchxml_with_page_size(self):
         """fetchxml() should pass page_size through to _query_fetchxml."""
         self.client._odata._query_fetchxml.return_value = iter([])
+        self.client._odata._extract_entity_from_fetchxml.return_value = "account"
 
         fetchxml = "<fetch><entity name='account' /></fetch>"
         list(self.client.query.fetchxml(fetchxml, page_size=50))
@@ -82,21 +88,26 @@ def test_fetchxml_with_page_size(self):
     def test_fetchxml_empty_result(self):
         """fetchxml() should return empty generator when no results."""
         self.client._odata._query_fetchxml.return_value = iter([])
+        self.client._odata._extract_entity_from_fetchxml.return_value = "account"
 
         fetchxml = "<fetch><entity name='account' /></fetch>"
         result = list(self.client.query.fetchxml(fetchxml))
 
         self.assertEqual(result, [])
 
     def test_fetchxml_returns_iterable(self):
-        """fetchxml() should return an iterable (generator)."""
+        """fetchxml() should return an iterable of Record pages."""
         self.client._odata._query_fetchxml.return_value = iter([[{"name": "A"}]])
+        self.client._odata._extract_entity_from_fetchxml.return_value = "account"
 
         fetchxml = "<fetch><entity name='account' /></fetch>"
         result = self.client.query.fetchxml(fetchxml)
 
         self.assertIsNotNone(iter(result))
-        self.assertEqual(list(result), [[{"name": "A"}]])
+        pages = list(result)
+        self.assertEqual(len(pages), 1)
+        self.assertIsInstance(pages[0][0], Record)
+        self.assertEqual(pages[0][0]["name"], "A")
 
 
 if __name__ == "__main__":