Issue 1023 (#1088)

* fixed bug that was setting to lowercase property names that were already in camelCase

* added datetime prefix to allow integration tests to be run repeatedly

* fixed test to use the ldap entries from ldap service in docker to test the service

* updated instructions for ldap test

* added comments to clarify the fix done in snake_to_camel method

Co-authored-by: Vijay Mariadassou <vijay@mariadassou.com>

docs/open-source-community/developer/run-integration-tests.md

@@ -39,9 +39,13 @@ pytest -s -c /dev/null
 ## Run LDAP test
 
 ```text
-cd ingestion
-source env/bin/activate
-cd tests/integration/ldap
+python3 -m venv /tmp/venv
+source /tmp/venv/bin/activate
+pip install -r ingestion/requirements.txt
+pip install -e ingestion
+pip install pytest
+pip install pytest-docker
+cd ingestion/tests/integration/ldap
 pytest -s -c /dev/null
 ```
 

ingestion/src/metadata/ingestion/models/json_serializable.py

@@ -27,9 +27,11 @@ class JsonSerializable(object, metaclass=abc.ABCMeta):
 
     @staticmethod
     def snake_to_camel(s):
+        # change property names like first_name to firstName
+        # property names that are already in camelCase like isBot are unchanged
         a = s.split("_")
-        a[0] = a[0].lower()
         if len(a) > 1:
+            a[0] = a[0].lower()
             a[1:] = [u.title() for u in a[1:]]
         return "".join(a)
 

ingestion/tests/integration/ldap/test_ldap_crud.py

@@ -14,7 +14,9 @@
 #  limitations under the License.
 
 import logging
-from time import sleep
+import time
+from datetime import datetime
+
 
 import pytest
 import requests
@@ -25,6 +27,13 @@ from metadata.ingestion.models.user import MetadataUser, User
 headers = {"Content-type": "application/json"}
 url = "http://localhost:8585/api/v1/users"
 
+def sleep(timeout_s):
+    print(f"sleeping for {timeout_s} seconds")
+    n = len(str(timeout_s))
+    for i in range(timeout_s, 0, -1):
+        print(f"{i:>{n}}", end="\r", flush=True)
+        time.sleep(1)
+    print(f"{'':>{n}}", end="\n", flush=True)
 
 def read_user_by_name(name: str):
     r = requests.get(url + "/name/" + name)
@@ -39,7 +48,6 @@ def status(r):
     else:
         return 0
 
-
 def ldap_connection():
     s = Server("ldap://localhost:389", get_info=ALL)
     c = Connection(s, user="cn=admin,dc=example,dc=com", password="ldappassword")
@@ -49,29 +57,51 @@ def ldap_connection():
         return False
     return [True, c]
 
+def is_ldap_listening(openldap_service):
+    c = openldap_service
+    if "listening" in str(c):
+        return True
 
 @pytest.fixture(scope="session")
 def openldap_service(docker_ip, docker_services):
     """Ensure that Docker service is up and responsive."""
-    sleep(10)
-    return ldap_connection()
+    port = docker_services.port_for("openldap", 389)
+    print(f"LDAP is running on port {port}")
+    timeout_s = 10
+    sleep(timeout_s)
+    conn = ldap_connection()[1]
+    docker_services.wait_until_responsive(
+        timeout=timeout_s, pause=0.1, check=lambda: is_ldap_listening(conn)
+    )
+    return conn
 
-
-def test_ldap_connection(openldap_service):
+@pytest.fixture(scope="session")
+def ldap_user_entry(openldap_service):
     c = openldap_service
-    if "listening" in str(c[1]):
-        assert 1
+    c.search('cn=John Doe,ou=users,dc=example,dc=com', '(objectclass=person)', attributes=['*'])
+    if c.entries:
+        return c.entries[0]
+    else:
+        logging.error("OpenLDAP not running")
+        assert 0
+
+@pytest.fixture(scope="session")
+def datetime_suffix():
+    # Openmetadata doesn't delete users; it deactivates them.
+    # Without changing the user's name with every run of this test,
+    # the test will fail due to duplicate user in metadata.
+    # Use a datetime suffix to have a new user name with every run.
+    now = datetime.now()
+    return now.strftime("_%Y%m%d%H%M%S")
 
 
-def test_insert_user(openldap_service):
-    c = openldap_service
-    if c[0]:
+def test_insert_user(ldap_user_entry, datetime_suffix):
     user = User(
-            "john_doe@example.com",
-            "John",
-            "Doe",
-            "John Doe",
-            "john_doe",
+        str(ldap_user_entry['mail']),
+        str(ldap_user_entry['givenName']),
+        str(ldap_user_entry['sn']),
+        str(ldap_user_entry['cn']),
+        str(ldap_user_entry['uid']) + datetime_suffix,
         "",
         "",
         "",
@@ -87,35 +117,25 @@ def test_insert_user(openldap_service):
         assert 1
     else:
         assert 0
-    else:
-        logging.error("OpenLDAP not running")
-        assert 0
 
 
-def test_read_user():
-    assert read_user_by_name("john_doe")[0]
+def test_read_user(ldap_user_entry, datetime_suffix):
+    assert read_user_by_name(str(ldap_user_entry['uid']) + datetime_suffix)[0]
 
 
-def test_update_user(openldap_service):
-    c = openldap_service
-    if c[0]:
-        user = read_user_by_name("john_doe")
+def test_update_user(ldap_user_entry, datetime_suffix):
+    user = read_user_by_name(str(ldap_user_entry['uid']) + datetime_suffix)
     user[1]["displayName"] = "Jane Doe"
     metadata_user = MetadataUser(
         name=user[1]["name"],
         display_name=user[1]["displayName"],
         email=user[1]["name"],
     )
-
     r = requests.patch(url, data=metadata_user.to_json(), headers=headers)
 
 
-def test_delete_user(openldap_service):
-    c = openldap_service
-    if c[0]:
-        r = read_user_by_name("john_doe")
+def test_delete_user(ldap_user_entry, datetime_suffix):
+    r = read_user_by_name(str(ldap_user_entry['uid']) + datetime_suffix)
     r = requests.delete(url + "/{}".format(r[1]["id"]))
     r.raise_for_status()
     assert 1
-    else:
-        assert 0