Trying to fix LDAP issues on production. Changing LDAP to static only methods, caching the connection and calling bind before all connection requests.

Also assuring we don't load the documents.xls file over and over again.

crc/api/approval.py

@@ -24,8 +24,7 @@ def get_approvals(everything=False):
 
 def get_approvals_for_study(study_id=None):
     db_approvals = ApprovalService.get_approvals_for_study(study_id)
-    ldap_service = LdapService()
-    approvals = [Approval.from_model(approval_model, ldap_service) for approval_model in db_approvals]
+    approvals = [Approval.from_model(approval_model) for approval_model in db_approvals]
     results = ApprovalSchema(many=True).dump(approvals)
     return results
 
@@ -37,7 +36,6 @@ def get_csv():
     approvals = ApprovalService.get_all_approvals(include_cancelled=False)
     output = []
     errors = []
-    ldapService = LdapService()
     for approval in approvals:
         try:
             if approval.status != ApprovalStatus.APPROVED.value:
@@ -53,12 +51,12 @@ def get_csv():
             pi_supervisor = extract_value(last_task, 'PISupervisor')['value']
             review_complete = 'AllRequiredTraining' in training_val
             pi_uid = workflow.study.primary_investigator_id
-            pi_details = ldapService.user_info(pi_uid)
+            pi_details = LdapService.user_info(pi_uid)
             details = []
             details.append(pi_details)
             for person in personnel:
                 uid = person['PersonnelComputingID']['value']
-                details.append(ldapService.user_info(uid))
+                details.append(LdapService.user_info(uid))
 
             for person in details:
                 record = {

crc/api/user.py

@@ -59,10 +59,7 @@ def sso_login():
     app.logger.info("SSO_LOGIN: Full URL: " + request.url)
     app.logger.info("SSO_LOGIN: User Id: " + uid)
     app.logger.info("SSO_LOGIN: Will try to redirect to : " + str(redirect))
-
-    ldap_service = LdapService()
-    info = ldap_service.user_info(uid)
-
+    info = LdapService.user_info(uid)
     return _handle_login(info, redirect)
 
 @app.route('/sso')
@@ -151,7 +148,7 @@ def backdoor(
    """
     if not 'PRODUCTION' in app.config or not app.config['PRODUCTION']:
 
-        ldap_info = LdapService().user_info(uid)
+        ldap_info = LdapService.user_info(uid)
         return _handle_login(ldap_info, redirect)
     else:
         raise ApiError('404', 'unknown')

crc/models/approval.py

@@ -53,7 +53,7 @@ class Approval(object):
         self.__dict__.update(kwargs)
 
     @classmethod
-    def from_model(cls, model: ApprovalModel, ldap_service: LdapSchema):
+    def from_model(cls, model: ApprovalModel):
         # TODO: Reduce the code by iterating over model's dict keys
         instance = cls()
         instance.id = model.id
@@ -72,12 +72,12 @@ class Approval(object):
         if model.study:
             instance.title = model.study.title
         try:
-            instance.approver = ldap_service.user_info(model.approver_uid)
-            instance.primary_investigator = ldap_service.user_info(model.study.primary_investigator_id)
+            instance.approver = LdapService.user_info(model.approver_uid)
+            instance.primary_investigator = LdapService.user_info(model.study.primary_investigator_id)
         except ApiError as ae:
             app.logger.error("Ldap lookup failed for approval record %i" % model.id)
 
-        doc_dictionary = FileService.get_reference_data(FileService.DOCUMENT_LIST, 'code', ['id'])
+        doc_dictionary = FileService.get_doc_dictionary()
         instance.associated_files = []
         for approval_file in model.approval_files:
             try:

crc/services/approval_service.py

@@ -28,12 +28,11 @@ class ApprovalService(object):
             query=query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value)
 
         approvals = query.all()
-        ldap_service = LdapService()
         for approval_model in approvals:
             if approval_model.approver_uid == approver_uid:
-                main_approval = Approval.from_model(approval_model, ldap_service)
+                main_approval = Approval.from_model(approval_model)
             else:
-                related_approvals.append(Approval.from_model(approval_model, ldap_service))
+                related_approvals.append(Approval.from_model(approval_model))
         if not main_approval and len(related_approvals) > 0:
             main_approval = related_approvals[0]
             related_approvals = related_approvals[1:]
@@ -70,12 +69,11 @@ class ApprovalService(object):
     def get_approvals_for_study(study_id, include_cancelled=True):
         """Returns an array of Approval objects for the study, it does not
          compute the related approvals."""
-        ldap_service = LdapService()
         query = session.query(ApprovalModel).filter_by(study_id=study_id)
         if not include_cancelled:
             query = query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value)
         db_approvals = query.all()
-        return [Approval.from_model(approval_model, ldap_service) for approval_model in db_approvals]
+        return [Approval.from_model(approval_model) for approval_model in db_approvals]
 
 
     @staticmethod

crc/services/file_service.py

@@ -45,10 +45,8 @@ class FileService(object):
 
     @staticmethod
     def is_allowed_document(code):
-        data_model = FileService.get_reference_file_data(FileService.DOCUMENT_LIST)
-        xls = ExcelFile(data_model.data)
-        df = xls.parse(xls.sheet_names[0])
-        return code in df['code'].values
+        doc_dict = FileService.get_doc_dictionary()
+        return code in doc_dict
 
     @staticmethod
     def add_workflow_file(workflow_id, irb_doc_code, name, content_type, binary_data):
@@ -333,4 +331,4 @@ class FileService(object):
             file_model = session.query(FileModel).filter_by(id=file_id).first()
             file_model.archived = True
             session.commit()
-            app.logger.error("Failed to delete file, so archiving it instead. %i, due to %s" % (file_id, str(ie)))
+            app.logger.info("Failed to delete file, so archiving it instead. %i, due to %s" % (file_id, str(ie)))

crc/services/ldap_service.py

@@ -19,37 +19,42 @@ class LdapService(object):
     cn_single_search = '(&(objectclass=person)(cn=%s*))'
     cn_double_search = '(&(objectclass=person)(&(cn=%s*)(cn=*%s*)))'
 
-    def __init__(self):
-        if app.config['TESTING']:
-            server = Server('my_fake_server')
-            self.conn = Connection(server, client_strategy=MOCK_SYNC)
-            file_path = os.path.abspath(os.path.join(app.root_path, '..', 'tests', 'data', 'ldap_response.json'))
-            self.conn.strategy.entries_from_json(file_path)
-            self.conn.bind()
-        else:
-            server = Server(app.config['LDAP_URL'], connect_timeout=app.config['LDAP_TIMEOUT_SEC'])
-            self.conn = Connection(server,
-                                   auto_bind=True,
-                                   receive_timeout=app.config['LDAP_TIMEOUT_SEC'],
-                                   )
+    conn = None
 
-    def __del__(self):
-        if self.conn:
-            self.conn.unbind()
+    @staticmethod
+    def __get_conn():
+        if not LdapService.conn:
+            if app.config['TESTING']:
+                server = Server('my_fake_server')
+                conn = Connection(server, client_strategy=MOCK_SYNC)
+                file_path = os.path.abspath(os.path.join(app.root_path, '..', 'tests', 'data', 'ldap_response.json'))
+                conn.strategy.entries_from_json(file_path)
+            else:
+                server = Server(app.config['LDAP_URL'], connect_timeout=app.config['LDAP_TIMEOUT_SEC'])
+                conn = Connection(server,
+                                       receive_timeout=app.config['LDAP_TIMEOUT_SEC'],
+                                       )
+            LdapService.conn = conn
+        return LdapService.conn
 
-    def user_info(self, uva_uid):
+
+    @staticmethod
+    def user_info(uva_uid):
         user_info = db.session.query(LdapModel).filter(LdapModel.uid == uva_uid).first()
         if not user_info:
             search_string = LdapService.uid_search_string % uva_uid
-            self.conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes)
-            if len(self.conn.entries) < 1:
+            conn = LdapService.__get_conn()
+            conn.bind()
+            conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes)
+            if len(conn.entries) < 1:
                 raise ApiError("missing_ldap_record", "Unable to locate a user with id %s in LDAP" % uva_uid)
-            entry = self.conn.entries[0]
+            entry = conn.entries[0]
             user_info = LdapModel.from_entry(entry)
             db.session.add(user_info)
         return user_info
 
-    def search_users(self, query, limit):
+    @staticmethod
+    def search_users(query, limit):
         if len(query.strip()) < 3:
             return []
         elif query.endswith(' '):
@@ -66,12 +71,14 @@ class LdapService(object):
         results = []
         print(search_string)
         try:
-            self.conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes)
+            conn = LdapService.__get_conn()
+            conn.bind()
+            conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes)
             # Entries are returned as a generator, accessing entries
             # can make subsequent calls to the ldap service, so limit
             # those here.
             count = 0
-            for entry in self.conn.entries:
+            for entry in conn.entries:
                 if count > limit:
                     break
                 results.append(LdapSchema().dump(LdapModel.from_entry(entry)))

crc/services/lookup_service.py

@@ -189,7 +189,7 @@ class LookupService(object):
 
     @staticmethod
     def _run_ldap_query(query, limit):
-        users = LdapService().search_users(query, limit)
+        users = LdapService.search_users(query, limit)
 
         """Converts the user models into something akin to the
         LookupModel in models/file.py, so this can be returned in the same way 

crc/services/study_service.py

@@ -26,7 +26,6 @@ from crc.models.approval import Approval
 
 class StudyService(object):
     """Provides common tools for working with a Study"""
-    ldap_service = LdapService()
 
     @staticmethod
     def get_studies_for_user(user):
@@ -207,7 +206,7 @@ class StudyService(object):
     @staticmethod
     def get_ldap_dict_if_available(user_id):
         try:
-            return LdapSchema().dump(StudyService.ldap_service.user_info(user_id))
+            return LdapSchema().dump(LdapService().user_info(user_id))
         except ApiError as ae:
             app.logger.info(str(ae))
             return {"error": str(ae)}

tests/test_ldap_service.py

@@ -7,13 +7,13 @@ from crc.services.ldap_service import LdapService
 class TestLdapService(BaseTest):
 
     def setUp(self):
-        self.ldap_service = LdapService()
+        pass
 
     def tearDown(self):
         pass
 
     def test_get_single_user(self):
-        user_info = self.ldap_service.user_info("lb3dp")
+        user_info = LdapService.user_info("lb3dp")
         self.assertIsNotNone(user_info)
         self.assertEqual("lb3dp", user_info.uid)
         self.assertEqual("Laura Barnes", user_info.display_name)
@@ -27,7 +27,7 @@ class TestLdapService(BaseTest):
 
     def test_find_missing_user(self):
         try:
-            user_info = self.ldap_service.user_info("nosuch")
+            user_info = LdapService.user_info("nosuch")
             self.assertFalse(True, "An API error should be raised.")
         except ApiError as ae:
             self.assertEquals("missing_ldap_record", ae.code)