Use ffi.gc to handle object lifecycle (#5)

Author: bbayles

Diff for: README.rst

@@ -40,9 +40,9 @@ This package exposes a ``URL`` class that is intended to match the one described
 .. code-block:: python
 
     >>> import ada_url
-    >>> with ada_url.URL('https://example.org/path/../file.txt') as urlobj:
-    ...     urlobj.host = 'example.com'
-    ...     new_url = urlobj.href
+    >>> ada_url.URL('https://example.org/path/../file.txt') as urlobj:
+    >>> urlobj.host = 'example.com'
+    >>> new_url = urlobj.href
     >>> new_url
     'https://example.com/file.txt'
 

Diff for: ada_url/ada_adapter.py

@@ -18,6 +18,12 @@
 SET_ATTRIBUTES = frozenset(URL_ATTRIBUTES)
 
 
+def _get_urlobj(constructor, *args):
+    urlobj = constructor(*args)
+
+    return ffi.gc(urlobj, lib.ada_free)
+
+
 def _get_str(x):
     ret = ffi.string(x.data, x.length).decode('utf-8') if x.length else ''
     return ret
@@ -32,19 +38,14 @@ class URL:
 
         >>> from ada_url import URL
         >>> old_url = 'https://example.org:443/file.txt?q=1'
-        >>> with URL(old_url) as urlobj:
-        ...     old_host = urlobj.host
-        ...     urlobj.host = 'example.com'
-        ...     new_url = urlobj.href
-        >>> old_host
+        >>> urlobj = URL(old_url)
+        >>> urlobj.host
         'example.org'
+        >>> urlobj.host = 'example.com'
+        >>> new_url = urlobj.href
         >>> new_url
         'https://example.com:443/file.txt?q=1'
 
-    Note that you should use this class as a context manager to ensure
-    that resources are freed. If you use it without a ``with``
-    statement, call the ``close()`` method manually.
-
     You can read and write the following attributes:
 
     * ``href``
@@ -78,11 +79,15 @@ def __init__(self, url, base=None):
         url_bytes = url.encode('utf-8')
 
         if base is None:
-            self.urlobj = lib.ada_parse(url_bytes, len(url_bytes))
+            self.urlobj = _get_urlobj(lib.ada_parse, url_bytes, len(url_bytes))
         else:
             base_bytes = base.encode('utf-8')
-            self.urlobj = lib.ada_parse_with_base(
-                url_bytes, len(url_bytes), base_bytes, len(base_bytes)
+            self.urlobj = _get_urlobj(
+                lib.ada_parse_with_base,
+                url_bytes,
+                len(url_bytes),
+                base_bytes,
+                len(base_bytes),
             )
 
         if not lib.ada_is_valid(self.urlobj):
@@ -119,15 +124,6 @@ def __setattr__(self, attr, value):
 
         return super().__setattr__(attr, value)
 
-    def close(self):
-        lib.ada_free(self.urlobj)
-
-    def __enter__(self, *args, **kwargs):
-        return self
-
-    def __exit__(self, *args, **kwargs):
-        self.close()
-
     @staticmethod
     def can_parse(url, base=None):
         try:
@@ -166,11 +162,8 @@ def check_url(s):
     except Exception:
         return False
 
-    urlobj = lib.ada_parse(s_bytes, len(s_bytes))
-    try:
-        return lib.ada_is_valid(urlobj)
-    finally:
-        lib.ada_free(urlobj)
+    urlobj = _get_urlobj(lib.ada_parse, s_bytes, len(s_bytes))
+    return lib.ada_is_valid(urlobj)
 
 
 def join_url(base_url, s):
@@ -192,14 +185,13 @@ def join_url(base_url, s):
     except Exception:
         raise ValueError('Invalid URL') from None
 
-    urlobj = lib.ada_parse_with_base(s_bytes, len(s_bytes), base_bytes, len(base_bytes))
-    try:
-        if not lib.ada_is_valid(urlobj):
-            raise ValueError('Invalid URL') from None
+    urlobj = _get_urlobj(
+        lib.ada_parse_with_base, s_bytes, len(s_bytes), base_bytes, len(base_bytes)
+    )
+    if not lib.ada_is_valid(urlobj):
+        raise ValueError('Invalid URL') from None
 
-        return _get_str(lib.ada_get_href(urlobj))
-    finally:
-        lib.ada_free(urlobj)
+    return _get_str(lib.ada_get_href(urlobj))
 
 
 def normalize_url(s):
@@ -260,19 +252,16 @@ def parse_url(s, attributes=PARSE_ATTRIBUTES):
         raise ValueError('Invalid URL') from None
 
     ret = {}
-    urlobj = lib.ada_parse(s_bytes, len(s_bytes))
-    try:
-        if not lib.ada_is_valid(urlobj):
-            raise ValueError('Invalid URL') from None
+    urlobj = _get_urlobj(lib.ada_parse, s_bytes, len(s_bytes))
+    if not lib.ada_is_valid(urlobj):
+        raise ValueError('Invalid URL') from None
 
-        for attr in attributes:
-            get_func = getattr(lib, f'ada_get_{attr}')
-            data = get_func(urlobj)
-            ret[attr] = _get_str(data)
-            if attr == 'origin':
-                lib.ada_free_owned_string(data)
-    finally:
-        lib.ada_free(urlobj)
+    for attr in attributes:
+        get_func = getattr(lib, f'ada_get_{attr}')
+        data = get_func(urlobj)
+        ret[attr] = _get_str(data)
+        if attr == 'origin':
+            lib.ada_free_owned_string(data)
 
     return ret
 
@@ -300,26 +289,23 @@ def replace_url(s, **kwargs):
     except Exception:
         raise ValueError('Invalid URL') from None
 
-    urlobj = lib.ada_parse(s_bytes, len(s_bytes))
-    try:
-        if not lib.ada_is_valid(urlobj):
-            raise ValueError('Invalid URL') from None
+    urlobj = _get_urlobj(lib.ada_parse, s_bytes, len(s_bytes))
+    if not lib.ada_is_valid(urlobj):
+        raise ValueError('Invalid URL') from None
 
-        for attr in URL_ATTRIBUTES:
-            value = kwargs.get(attr)
-            if value is None:
-                continue
+    for attr in URL_ATTRIBUTES:
+        value = kwargs.get(attr)
+        if value is None:
+            continue
 
-            try:
-                value_bytes = value.encode()
-            except Exception:
-                raise ValueError(f'Invalid value for {attr}') from None
+        try:
+            value_bytes = value.encode()
+        except Exception:
+            raise ValueError(f'Invalid value for {attr}') from None
 
-            set_func = getattr(lib, f'ada_set_{attr}')
-            set_result = set_func(urlobj, value_bytes, len(value_bytes))
-            if (set_result is not None) and (not set_result):
-                raise ValueError(f'Invalid value for {attr}') from None
+        set_func = getattr(lib, f'ada_set_{attr}')
+        set_result = set_func(urlobj, value_bytes, len(value_bytes))
+        if (set_result is not None) and (not set_result):
+            raise ValueError(f'Invalid value for {attr}') from None
 
-        return _get_str(lib.ada_get_href(urlobj))
-    finally:
-        lib.ada_free(urlobj)
+    return _get_str(lib.ada_get_href(urlobj))

Diff for: tests/test_ada_url.py

@@ -14,55 +14,54 @@
 class ADAURLTests(TestCase):
     def test_class_get(self):
         url = 'https://user_1:[email protected]:8080/dir/../api?q=1#frag'
-        with URL(url) as urlobj:
-            self.assertEqual(
-                urlobj.href, 'https://user_1:[email protected]:8080/api?q=1#frag'
-            )
-            self.assertEqual(urlobj.username, 'user_1')
-            self.assertEqual(urlobj.password, 'password_1')
-            self.assertEqual(urlobj.protocol, 'https:')
-            self.assertEqual(urlobj.port, '8080')
-            self.assertEqual(urlobj.hostname, 'example.org')
-            self.assertEqual(urlobj.host, 'example.org:8080')
-            self.assertEqual(urlobj.pathname, '/api')
-            self.assertEqual(urlobj.search, '?q=1')
-            self.assertEqual(urlobj.hash, '#frag')
-            self.assertEqual(urlobj.origin, 'https://example.org:8080')
+        urlobj = URL(url)
+        self.assertEqual(
+            urlobj.href, 'https://user_1:[email protected]:8080/api?q=1#frag'
+        )
+        self.assertEqual(urlobj.username, 'user_1')
+        self.assertEqual(urlobj.password, 'password_1')
+        self.assertEqual(urlobj.protocol, 'https:')
+        self.assertEqual(urlobj.port, '8080')
+        self.assertEqual(urlobj.hostname, 'example.org')
+        self.assertEqual(urlobj.host, 'example.org:8080')
+        self.assertEqual(urlobj.pathname, '/api')
+        self.assertEqual(urlobj.search, '?q=1')
+        self.assertEqual(urlobj.hash, '#frag')
+        self.assertEqual(urlobj.origin, 'https://example.org:8080')
 
-            with self.assertRaises(AttributeError):
-                urlobj.bogus
+        with self.assertRaises(AttributeError):
+            urlobj.bogus
 
     def test_class_set(self):
         url = 'https://username:[email protected]:8080/'
-        with URL(url) as urlobj:
-            urlobj.href = 'https://www.yagiz.co'
-            urlobj.hash = 'new-hash'
-            urlobj.hostname = 'new-host'
-            urlobj.host = 'changed-host:9090'
-            urlobj.pathname = 'new-pathname'
-            urlobj.search = 'new-search'
-            urlobj.protocol = 'wss'
-            actual = urlobj.href
+        urlobj = URL(url)
+        urlobj.href = 'https://www.yagiz.co'
+        urlobj.hash = 'new-hash'
+        urlobj.hostname = 'new-host'
+        urlobj.host = 'changed-host:9090'
+        urlobj.pathname = 'new-pathname'
+        urlobj.search = 'new-search'
+        urlobj.protocol = 'wss'
+        actual = urlobj.href
 
-            with self.assertRaises(ValueError):
-                urlobj.hostname = 1
+        with self.assertRaises(ValueError):
+            urlobj.hostname = 1
 
-            with self.assertRaises(ValueError):
-                urlobj.hostname = '127.0.0.0.0.1'
+        with self.assertRaises(ValueError):
+            urlobj.hostname = '127.0.0.0.0.1'
 
         expected = 'wss://changed-host:9090/new-pathname?new-search#new-hash'
         self.assertEqual(actual, expected)
 
     def test_class_with_base(self):
         url = '../example.txt'
         base = 'https://example.org/path/'
-        with URL(url, base) as urlobj:
-            self.assertEqual(urlobj.href, 'https://example.org/example.txt')
+        urlobj = URL(url, base)
+        self.assertEqual(urlobj.href, 'https://example.org/example.txt')
 
     def test_class_invalid(self):
         with self.assertRaises(ValueError):
-            with URL('bogus'):
-                pass
+            URL('bogus')
 
     def test_class_can_parse(self):
         for url, expected in (
@@ -88,9 +87,8 @@ def test_class_can_parse_with_base(self):
                 self.assertEqual(actual, expected)
 
     def test_class_dir(self):
-        with URL('https://example.org') as urlobj:
-            actual = set(dir(urlobj))
-
+        urlobj = URL('https://example.org')
+        actual = set(dir(urlobj))
         self.assertTrue(actual.issuperset(GET_ATTRIBUTES))
 
     def test_check_url(self):