UID2-2760 Add IdentityMapClient by caroline-ttd · Pull Request #39 · IABTechLab/uid2-client-python

caroline-ttd · 2024-05-11T02:03:03Z

Added IdentityMapClient
Sample IdentityMapClient tests
IdentityMapClient Integration tests
Fixed make docker and remove broken commands in readme

examples/sample_identity_map_client.py

jon8787 · 2024-05-12T23:39:28Z

uid2_client/identity_map_client.py

+
+
+class IdentityMapClient:
+    """Client for interacting with UID2 Identity Map services


note that this class needs to work for EUID as well. Instead of UID2 in this file we can say UID2 and EUID or UID

I updated it to UID

caroline-ttd · 2024-05-15T22:58:37Z

uid2_client/identity_map_input.py

+            "email_hash": self.hashed_normalized_emails,
+            "phone_hash": self.hashed_normalized_phones
+        }
+        return json.dumps({k: v for k, v in json_object.items() if v is not None})


Initialize the variables hashed_normalized_emails and hashed_normalized_phones as None, and then set their values later. This approach allows us to create objects like {"email_hash":[]} instead of {} when the input is empty. Passing {} to the operator would result in a 400 bad request error.

tests/test_identity_map_client.py

examples/sample_identity_map_client.py

uid2_client/identity_map_client.py

asloobq · 2024-05-17T05:30:22Z

uid2_client/identity_map_input.py

+        if identity_type == IdentityType.Email:
+            self.hashed_normalized_emails = []
+            for email in emails_or_phones:
+                if already_hashed:


probably don't need to enter the for loop if already_hashed is true . You can set self.hashed_normalized_emails.append( emails_or_phones)

That makes sense. I change it to self.hashed_normalized_emails = emails_or_phones

self.hashed_normalized_emails.append( emails_or_phones) will add emails_or_phones as one element in the hashed_normalized_emails list.

asloobq · 2024-05-17T05:44:54Z

uid2_client/identity_map_input.py

@@ -0,0 +1,63 @@
+import json
+
+from uid2_client import IdentityType, normalize_email_string, get_base64_encoded_hash, is_phone_number_normalized, \


some imports appear unused

Good catch, thank you. I may copy paste from other class....

asloobq · 2024-05-17T05:47:55Z

examples/sample_identity_map_client.py

+# which contains raw uid or the reason why it is unmapped
+
+def _usage():
+    print('Usage: python3 sample_sharing_client.py <base_url> <api_key> <client_secret> <email_1> <email_2> ... <email_n>'


sample_identity_map_client.py instead of sample_sharing_client.py

My bad, good catch!

asloobq · 2024-05-17T05:49:05Z

examples/sample_identity_map_client.py

+from uid2_client import IdentityMapClient, IdentityMapInput
+
+
+# this sample client takes email addresses or phone numbers as input and generates an IdentityMapResponse object


I know this example demonstrates mapping emails only, so either we should mention only emails here or mention all 4 types including hashes of emails and phones

Yeah, I updated the description

asloobq · 2024-05-17T05:53:14Z

uid2_client/identity_map_response.py

+        if not self.is_success():
+            raise ValueError("Got unexpected identity map status: " + self.status)
+
+        body = self._get_body_as_json(response_json)


nitpicking here: I don't see much value in refactoring this into a separate method since its not called from anywhere else and its just one line

Yeah, I agree, I removed that method.

asloobq · 2024-05-17T06:00:27Z

uid2_client/identity_map_response.py

+
+class MappedIdentity:
+    def __init__(self, raw_uid, bucket_id):
+        self.raw_uid = raw_uid


to denote that properties should not be accessed directly it is a convention to start the name with an underscore. See EncryptionDataResponse for example

Thank you, I updated it.

probably also change the vars raw_uid and bucket_id to _raw_uid and _bucket_id

Yeah, that makes sense. I updated it.

asloobq · 2024-05-17T16:22:38Z

tests/test_identity_map_client.py

+        cls.UID2_API_KEY = os.getenv("UID2_API_KEY")
+        cls.UID2_SECRET_KEY = os.getenv("UID2_SECRET_KEY")
+
+        print(cls.UID2_BASE_URL, cls.UID2_API_KEY, cls.UID2_SECRET_KEY)


unnecessary print statement ?

Yeah, removed it.

asloobq · 2024-05-17T16:25:03Z

tests/test_identity_map_client.py

+        self.assertRaises(HTTPError, client.generate_identity_map,
+                          identity_map_input)
+
+    def assert_mapped(self, response, ddi):


dii instead of ddi ?

Thank you, I updated it.

jon8787 · 2024-05-21T12:18:26Z

examples/sample_identity_map_client.py

-# this sample client takes email addresses or phone numbers as input and generates an IdentityMapResponse object
-# which contains raw uid or the reason why it is unmapped
+# this sample client takes email addresses or phone numbers or hashed versions of either as input and generates
+# an IdentityMapResponse object which contains raw uid or the reason why it is unmapped


this comment doesn't match what's in _usage()

I updated the comment

jon8787 · 2024-05-21T12:58:33Z

uid2_client/identity_map_input.py

+                    self._add_hashed_to_raw_dii_mapping(hashed_normalized_email, email)
+                    self.hashed_normalized_emails.append(hashed_normalized_email)
+        else:  # phone
+            self.hashed_normalized_phones = []


could the above line go inside the else block at line 27?

Yeah, I moved it.

asloobq

LGTM 👍

jon8787 · 2024-05-22T00:00:28Z

uid2_client/identity_map_input.py

+                    self._add_hashed_to_raw_dii_mapping(hashed_normalized_email, email)
+                    self.hashed_normalized_emails.append(hashed_normalized_email)
+        else:  # phone
+            if already_hashed:


nit: if already_hashed block should not depend on whether it's an email or phone and should go outside the phone/email check

Hmm, it depends on the type. If it's a hashed email list, it gets assigned to hashed_normalized_emails, and if it's a hashed phone list, it's assigned to hashed_normalized_phones.

Add IdentityMapClient

28ab011

caroline-ttd changed the title ~~Add IdentityMapClient~~ UID2-2760 Add IdentityMapClient May 11, 2024

jon8787 reviewed May 12, 2024

View reviewed changes

caroline-ttd added 3 commits May 15, 2024 10:23

Add unit tests

7dcafc3

Address the comments

7f29ce4

Update the unit tests

bcf8224

caroline-ttd marked this pull request as ready for review May 15, 2024 22:25

caroline-ttd commented May 15, 2024

View reviewed changes

tests/test_identity_map_client.py Show resolved Hide resolved

caroline-ttd added 2 commits May 15, 2024 18:56

Fix 'make docker' command and remove broken commands in the readme

7a33fe1

Update Readme

3a9e2de

jon8787 reviewed May 16, 2024

View reviewed changes

examples/sample_identity_map_client.py Outdated Show resolved Hide resolved

jon8787 reviewed May 16, 2024

View reviewed changes

uid2_client/identity_map_client.py Outdated Show resolved Hide resolved

Address comments

75d759e

caroline-ttd requested review from asloobq and jon8787 May 16, 2024 22:02

asloobq reviewed May 17, 2024

View reviewed changes

Address the comments

0d2e396

jon8787 approved these changes May 21, 2024

View reviewed changes

jon8787 reviewed May 21, 2024

View reviewed changes

Address the comments

ff74116

asloobq approved these changes May 21, 2024

View reviewed changes

jon8787 reviewed May 22, 2024

View reviewed changes

caroline-ttd merged commit 3740fd4 into main May 22, 2024

caroline-ttd deleted the ccm-UID2-2760-implement-identity-map branch May 22, 2024 02:21



		class IdentityMapClient:
		"""Client for interacting with UID2 Identity Map services

		@@ -0,0 +1,63 @@
		import json

		from uid2_client import IdentityType, normalize_email_string, get_base64_encoded_hash, is_phone_number_normalized, \

		from uid2_client import IdentityMapClient, IdentityMapInput


		# this sample client takes email addresses or phone numbers as input and generates an IdentityMapResponse object

Conversation

caroline-ttd commented May 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jon8787 May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asloobq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

caroline-ttd commented May 11, 2024 •

edited

Loading

jon8787 May 21, 2024 •

edited

Loading