Metadata query params #29

kleinjm · 2021-08-11T23:14:20Z

What

Generating the SDK after changes to the swagger.json file
Adds ability to query orders index with metadata
Fixes the creation of orders with metadata

Why

Clients want the ability to query orders via metadata.

SDK Release Checklist

Have you added an integration test for the changes?
Have you built the library locally and made queries against it successfully?
Did you update the changelog?
Did you bump the package version?
For breaking changes, did you plan for the release of the new SDK versions and deploy the API to production?

kleinjm · 2021-08-17T16:33:50Z

patch_api/api/orders_api.py

+        # do not add duplicate keys to query_params list
+        existing_keys = []
+        for param in query_params:
+            existing_keys.append(param[0])
+
+        for key in kwargs:
+            if key not in existing_keys:
+                query_params.append([key, kwargs.get(key)])


this is the place in which this new logic is most relevant. Previously, a named query param would appear twice because all args were added to the params list even if they already matched one of the conditionals above

kleinjm · 2021-08-17T19:27:51Z

patch_api/rest.py

+    def recursive_urlencode(self, d):
+        """URL-encode a multidimensional dictionary.
+
+        >>> data = {'a': 'b&c', 'd': {'e': {'f&g': 'h*i'}}, 'j': 'k'}
+        >>> recursive_urlencode(data)
+        u'a=b%26c&j=k&d[e][f%26g]=h%2Ai'
+        """
+
+        def recursion(d, base=[]):
+            pairs = []
+
+            for key, value in d.items():
+                new_base = base + [key]
+                if hasattr(value, "values"):
+                    pairs += recursion(value, new_base)
+                else:
+                    new_pair = None
+                    if len(new_base) > 1:
+                        first = urllib.parse.quote(new_base.pop(0))
+                        rest = map(lambda x: urllib.parse.quote(x), new_base)
+                        new_pair = "%s[%s]=%s" % (
+                            first,
+                            "][".join(rest),
+                            urllib.parse.quote(str(value)),
+                        )
+                    else:
+                        new_pair = "%s=%s" % (
+                            urllib.parse.quote(str(key)),
+                            urllib.parse.quote(str(value)),
+                        )
+                    pairs.append(new_pair)
+            return pairs
+
+        return "&".join(recursion(d))
+
+    def encoded_query_params(self, query_params):
+        if not query_params:
+            return ""
+
+        final_query_params = ""
+        for key, value in query_params:
+            if isinstance(value, dict):
+                nested_param = {}
+                nested_param[key] = value
+                final_query_params += self.recursive_urlencode(nested_param)
+                query_params.remove((key, value))
+
+        if query_params:
+            final_query_params += "&" + urlencode(query_params)
+
+        return "?" + final_query_params


this is all custom code. I took recursive_urlencode from a stackoverflow answer and modified it to work with python3.
encoded_query_params is a method I wrote

Is there a chance we could unit test those two methods? I trust the global test but this might save some pain later.

kleinjm · 2021-08-17T19:28:14Z

test/test_estimates_api.py

+        self.assertEqual(estimate.data.order.mass_g, 1000622)
+        self.assertEqual(estimate.data.mass_g, 1000622)


I confirmed this change is correct with Brendan

kleinjm · 2021-08-17T19:28:18Z

test/test_estimates_api.py

@@ -75,7 +75,7 @@ def test_create_and_retrieve_shipping_estimate(self):
        )
        self.assertEqual(estimate.data.order, None)
        self.assertEqual(estimate.data.type, "shipping")
-        self.assertEqual(estimate.data.mass_g, 373)
+        self.assertEqual(estimate.data.mass_g, 249)


I confirmed this change is correct with Brendan

pcothenet · 2021-08-17T22:09:51Z

CHANGELOG.md

+
+### Added
+
+- Fixed ability to create Orders with `metadata`


pcothenet

Looks good overall. This library does like repeating itself though.

Open question about unit-testing the query params method.

pcothenet · 2021-08-17T22:10:04Z

CHANGELOG.md

+### Added
+
+- Fixed ability to create Orders with `metadata`
+- Add support for querying Orders by `metadata`


Can you add the new ETH parameter here as well?

pcothenet · 2021-08-17T22:11:08Z

patch_api/api/estimates_api.py

+
+        # do not add duplicate keys to query_params list
+        existing_keys = []
+        for param in query_params:
+            existing_keys.append(param[0])
+
        for key in kwargs:
-            query_params.append([key, kwargs.get(key)])
+            if key not in existing_keys:
+                query_params.append([key, kwargs.get(key)])


We'll probably want to address the fact that this is duplicated several times over?

I suppose but this logic does seem useful to have in each case so there aren't duplicate params in the list

pcothenet · 2021-08-17T22:14:10Z

patch_api/rest.py

+    def recursive_urlencode(self, d):
+        """URL-encode a multidimensional dictionary.
+
+        >>> data = {'a': 'b&c', 'd': {'e': {'f&g': 'h*i'}}, 'j': 'k'}
+        >>> recursive_urlencode(data)
+        u'a=b%26c&j=k&d[e][f%26g]=h%2Ai'
+        """
+
+        def recursion(d, base=[]):
+            pairs = []
+
+            for key, value in d.items():
+                new_base = base + [key]
+                if hasattr(value, "values"):
+                    pairs += recursion(value, new_base)
+                else:
+                    new_pair = None
+                    if len(new_base) > 1:
+                        first = urllib.parse.quote(new_base.pop(0))
+                        rest = map(lambda x: urllib.parse.quote(x), new_base)
+                        new_pair = "%s[%s]=%s" % (
+                            first,
+                            "][".join(rest),
+                            urllib.parse.quote(str(value)),
+                        )
+                    else:
+                        new_pair = "%s=%s" % (
+                            urllib.parse.quote(str(key)),
+                            urllib.parse.quote(str(value)),
+                        )
+                    pairs.append(new_pair)
+            return pairs
+
+        return "&".join(recursion(d))
+
+    def encoded_query_params(self, query_params):
+        if not query_params:
+            return ""
+
+        final_query_params = ""
+        for key, value in query_params:
+            if isinstance(value, dict):
+                nested_param = {}
+                nested_param[key] = value
+                final_query_params += self.recursive_urlencode(nested_param)
+                query_params.remove((key, value))
+
+        if query_params:
+            final_query_params += "&" + urlencode(query_params)
+
+        return "?" + final_query_params


Is there a chance we could unit test those two methods? I trust the global test but this might save some pain later.

kleinjm · 2021-08-18T00:19:40Z

test/test_rest.py

+        encoded_params = client.encoded_query_params([('mass_g', 100)])
+        self.assertEqual(encoded_params, "?mass_g=100")


@pcothenet unit tests added 🥳 and they caught a small issue of adding an extra & to the url

update sdk post-addition of metadata query param

6f1c0a1

kleinjm requested review from pcothenet and removed request for pcothenet August 11, 2021 23:14

kleinjm added 4 commits August 13, 2021 18:47

update from latest swagger json

8cf4dfd

create working, index still failing

bd9c101

query params working for create and index

2d2b24c

regenerated sdk

eb8fb5e

kleinjm commented Aug 17, 2021

View reviewed changes

kleinjm added 4 commits August 17, 2021 10:36

remove ignored files

99da46d

bump version and update changelog

dd1260b

fix tests

120a79c

lint

38080fb

kleinjm commented Aug 17, 2021

View reviewed changes

kleinjm requested a review from pcothenet August 17, 2021 19:29

pcothenet reviewed Aug 17, 2021

View reviewed changes

CHANGELOG.md

### Added

- Fixed ability to create Orders with `metadata`

Copy link

Contributor

pcothenet Aug 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

pcothenet approved these changes Aug 17, 2021

View reviewed changes

update from swagger json, add to changelog, and add unit tests

ae1bfcc

kleinjm commented Aug 18, 2021

View reviewed changes

lint

3d146f3

kleinjm merged commit 968663f into main Aug 18, 2021

kleinjm deleted the jklein/metadata_query branch August 18, 2021 17:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metadata query params #29

Metadata query params #29

kleinjm commented Aug 11, 2021 •

edited

Loading

kleinjm Aug 17, 2021

kleinjm Aug 17, 2021

pcothenet Aug 17, 2021

kleinjm Aug 17, 2021

kleinjm Aug 17, 2021

pcothenet Aug 17, 2021

pcothenet left a comment

pcothenet Aug 17, 2021

pcothenet Aug 17, 2021

kleinjm Aug 17, 2021

pcothenet Aug 17, 2021

kleinjm Aug 18, 2021

		self.assertEqual(estimate.data.order.mass_g, 1000622)
		self.assertEqual(estimate.data.mass_g, 1000622)

		encoded_params = client.encoded_query_params([('mass_g', 100)])
		self.assertEqual(encoded_params, "?mass_g=100")

Metadata query params #29

Metadata query params #29

Conversation

kleinjm commented Aug 11, 2021 • edited Loading

What

Why

SDK Release Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcothenet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kleinjm commented Aug 11, 2021 •

edited

Loading