Merge pull request #111 from ServiceNow/scratch/iter

feat: non-rewindable

Author: vetsin

poetry.lock

@@ -18,10 +18,9 @@ classifiers = [
 
 [tool.poetry.dependencies]
 python = "^3.8"
-requests = "2.31.0"
+requests = "^2.31.0"
 requests-oauthlib = { version = ">=1.2.0", optional = true}
-certifi = "2023.7.22"
-pip = ">=23.3.1"
+certifi = "^2024.7.4"
 urllib3 = "^2.0.7"
 
 [tool.poetry.extras]

pyproject.toml

@@ -83,15 +83,19 @@ def __init__(self, instance, auth, proxy=None, verify=None, cert=None, auto_retr
         self.attachment_api = AttachmentAPI(self)
         self.batch_api = BatchAPI(self)
 
-    def GlideRecord(self, table, batch_size=100) -> GlideRecord:
+    def GlideRecord(self, table, batch_size=100, rewindable=True) -> GlideRecord:
         """
         Create a :class:`pysnc.GlideRecord` for a given table against the current client
 
         :param str table: The table name e.g. ``problem``
         :param int batch_size: Batch size (items returned per HTTP request). Default is ``100``.
+        :param bool rewindable: If we can rewind the record. Default is ``True``. If ``False`` then we cannot rewind 
+                                the record, which means as an Iterable this object will be 'spent' after iteration.
+                                This is normally the default behavior expected for a python Iterable, but not a GlideRecord.
+                                When ``False`` less memory will be consumed, as each previous record will be collected.
         :return: :class:`pysnc.GlideRecord`
         """
-        return GlideRecord(self, table, batch_size)
+        return GlideRecord(self, table, batch_size, rewindable)
 
     def Attachment(self, table) -> Attachment:
         """

pysnc/client.py

@@ -243,8 +243,12 @@ class GlideRecord(object):
     :param ServiceNowClient client: We need to know which instance we're connecting to
     :param str table: The table are we going to access
     :param int batch_size: Batch size (items returned per HTTP request). Default is ``500``.
+    :param bool rewindable: If we can rewind the record. Default is ``True``. If ``False`` then we cannot rewind 
+                            the record, which means as an Iterable this object will be 'spent' after iteration.
+                            This is normally the default behavior expected for a python Iterable, but not a GlideRecord.
+                            When ``False`` less memory will be consumed, as each previous record will be collected.
     """
-    def __init__(self, client: 'ServiceNowClient', table: str, batch_size=500):
+    def __init__(self, client: 'ServiceNowClient', table: str, batch_size: int=500, rewindable: bool=True):
         self._log = logging.getLogger(__name__)
         self._client = client
         self.__table: str = table
@@ -262,6 +266,7 @@ def __init__(self, client: 'ServiceNowClient', table: str, batch_size=500):
         self.__order: str = "ORDERBYsys_id" # we *need* a default order in the event we page, see issue#96
         self.__is_new_record: bool = False
         self.__display_value: Union[bool, str] = 'all'
+        self.__rewindable = rewindable
 
     def _clear_query(self):
         self.__query = Query(self.__table)
@@ -443,6 +448,7 @@ def order_by_desc(self, column: str):
     def pop_record(self) -> 'GlideRecord':
         """
         Pop the current record into a new :class:`GlideRecord` object - equivalent to a clone of a singular record
+        FIXME: this, by the name, should be a destructive operation, but it is not.
 
         :return: Give us a new :class:`GlideRecord` containing only the current record
         """
@@ -481,8 +487,10 @@ def set_new_guid_value(self, value):
 
     def rewind(self):
         """
-        Rewinds the record so it may be iterated upon again. Not required to be called if iterating in the pythonic method.
+        Rewinds the record (iterable) so it may be iterated upon again. Only possible when the record is rewindable.
         """
+        if not self._is_rewindable():
+            raise Exception('Cannot rewind a non-rewindable record')
         self.__current = -1
 
     def changes(self) -> bool:
@@ -506,6 +514,12 @@ def query(self, query=None):
             :AuthenticationException: If we do not have rights
             :RequestException: If the transaction is canceled due to execution time
         """
+        if not self._is_rewindable() and self.__current > 0:
+            raise RuntimeError(f"huh {self._is_rewindable} and {self.__current}")
+        #    raise RuntimeError('Cannot re-query a non-rewindable record that has been iterated upon')
+        self._do_query(query)
+
+    def _do_query(self, query=None):
         stored = self.__query
         if query:
             assert isinstance(query, Query), 'cannot query with a non query object'
@@ -564,7 +578,7 @@ def get(self, name, value=None) -> bool:
             return False
         else:
             self.add_query(name, value)
-            self.query()
+            self._do_query()
             return self.next()
 
     def insert(self) -> Optional[GlideElement]:
@@ -645,7 +659,7 @@ def delete_multiple(self) -> bool:
         if self.__total is None:
             if not self.__field_limits:
                 self.fields = 'sys_id'  # type: ignore  ## all we need...
-            self.query()
+            self._do_query()
 
         allRecordsWereDeleted = True
         def handle(response):
@@ -1074,9 +1088,13 @@ def to_pandas(self, columns=None, mode='smart'):
 
         return data
 
+    def _is_rewindable(self) -> bool:
+        return self.__rewindable
+
     def __iter__(self):
         self.__is_iter = True
-        self.rewind()
+        if self._is_rewindable():
+            self.rewind()
         return self
 
     def __next__(self):
@@ -1092,6 +1110,8 @@ def next(self, _recursive=False) -> bool:
         if l > 0 and self.__current+1 < l:
             self.__current = self.__current + 1
             if self.__is_iter:
+                if not self._is_rewindable(): # if we're not rewindable, remove the previous record
+                    self.__results[self.__current - 1] = None
                 return self  # type: ignore  # this typing is internal only
             return True
         if self.__total and self.__total > 0 and \
@@ -1100,10 +1120,10 @@ def next(self, _recursive=False) -> bool:
                 _recursive is False:
             if self.__limit:
                 if self.__current+1 < self.__limit:
-                    self.query()
+                    self._do_query()
                     return self.next(_recursive=True)
             else:
-                self.query()
+                self._do_query()
                 return self.next(_recursive=True)
         if self.__is_iter:
             self.__is_iter = False

pysnc/record.py

@@ -1,4 +1,4 @@
-from unittest import TestCase
+from unittest import TestCase, skip
 
 from pysnc import ServiceNowClient
 from pysnc.auth import *
@@ -29,6 +29,7 @@ def test_basic_fail(self):
         except Exception:
             assert 'Should have got an Auth exception'
 
+    @skip("Requires valid oauth client_id and secret, and I don't want to need anything not out of box")
     def test_oauth(self):
         # Manual setup using legacy oauth
         server = self.c.server

test/test_snc_auth.py

@@ -0,0 +1,57 @@
+from unittest import TestCase
+
+from pysnc import ServiceNowClient
+from constants import Constants
+from pprint import pprint
+
+class TestIteration(TestCase):
+    c = Constants()
+
+    def test_default_behavior(self):
+        client = ServiceNowClient(self.c.server, self.c.credentials)
+        gr = client.GlideRecord('sys_metadata', batch_size=100)
+        gr.fields = 'sys_id'
+        gr.limit = 500
+        gr.query()
+        self.assertTrue(gr._is_rewindable())
+
+        self.assertTrue(len(gr) > 500, 'Expected more than 500 records')
+
+        count = 0
+        while gr.next():
+            count += 1
+        self.assertEqual(count, 500, 'Expected 500 records when using next')
+
+        self.assertEqual(len([r.sys_id for r in gr]), 500, 'Expected 500 records when an iterable')
+        self.assertEqual(len([r.sys_id for r in gr]), 500, 'Expected 500 records when iterated again')
+
+        # expect the same for next
+        count = 0
+        while gr.next():
+            count += 1
+        self.assertEqual(count, 0, 'Expected 0 records when not rewound, as next does not auto-rewind')
+        gr.rewind()
+        while gr.next():
+            count += 1
+        self.assertEqual(count, 500, 'Expected 500 post rewind')
+
+        # should not throw
+        gr.query()
+        gr.query()
+
+        client.session.close()
+
+    def test_rewind_behavior(self):
+        client = ServiceNowClient(self.c.server, self.c.credentials)
+        gr = client.GlideRecord('sys_metadata', batch_size=250, rewindable=False)
+        gr.fields = 'sys_id'
+        gr.limit = 500
+        gr.query()
+        self.assertEqual(gr._GlideRecord__current, -1)
+        self.assertFalse(gr._is_rewindable())
+        self.assertEqual(len([r for r in gr]), 500, 'Expected 500 records when an iterable')
+        self.assertEqual(len([r for r in gr]), 0, 'Expected no records when iterated again')
+
+        # but if we query again...
+        with self.assertRaises(RuntimeError):
+            gr.query()