Merge pull request #35002 from dimagi/mk/3822-refactor

Refactors around `ConfigurableAPI`

corehq/apps/userreports/expressions/factory.py

@@ -68,7 +68,6 @@ def _simple_expression_generator(wrapper_class, spec, factory_context):
 
 _identity_expression = functools.partial(_simple_expression_generator, IdentityExpressionSpec)
 _constant_expression = functools.partial(_simple_expression_generator, ConstantGetterSpec)
-_property_name_expression = functools.partial(_simple_expression_generator, PropertyNameGetterSpec)
 _property_path_expression = functools.partial(_simple_expression_generator, PropertyPathGetterSpec)
 _iteration_number_expression = functools.partial(_simple_expression_generator, IterationNumberExpressionSpec)
 _jsonpath_expression = functools.partial(_simple_expression_generator, JsonpathExpressionSpec)

corehq/motech/generic_inbound/backend/base.py

@@ -26,8 +26,8 @@ def get_basic_error_response(cls, request_id, status_code, message):
         It can also be used for basic errors that don't require additional context."""
         raise NotImplementedError
 
-    def __init__(self, api_model, request_data):
-        self.api_model = api_model
+    def __init__(self, configurable_api, request_data):
+        self.configurable_api = configurable_api
         self.request_data = request_data
 
     def run(self):
@@ -37,7 +37,7 @@ def run(self):
                 self.request_data.couch_user,
                 self.request_data.user_agent,
                 self.get_context(),
-                self.api_model,
+                self.configurable_api,
             )
             return self.get_success_response(response_json)
         except GenericInboundRequestFiltered:
@@ -60,11 +60,11 @@ def run(self):
 
     def get_context(self):
         return get_evaluation_context(
-            self.request_data.restore_user,
-            self.request_data.request_method,
-            self.request_data.query,
-            self.request_data.headers,
-            self._get_body_for_eval_context()
+            restore_user=self.request_data.restore_user,
+            method=self.request_data.request_method,
+            query=self.request_data.query,
+            headers=self.request_data.headers,
+            body=self._get_body_for_eval_context()
         )
 
     def get_success_response(self, response_json):
@@ -90,11 +90,12 @@ def _get_validation_error(self, status_code, message, errors):
         raise NotImplementedError
 
 
-def _execute_generic_api(domain, couch_user, device_id, context, api_model):
-    _apply_api_filter(api_model, context)
-    _validate_api_request(api_model, context)
+def _execute_generic_api(domain, couch_user, device_id, context, configurable_api):
+    # context is all data returned in structure corehq.motech.generic_inbound.utils.get_evaluation_context
+    _apply_api_filter(configurable_api, context)
+    _validate_api_request(configurable_api, context)
 
-    data = api_model.parsed_expression(context.root_doc, context)
+    data = configurable_api.parsed_expression(context.root_doc, context)
 
     if not isinstance(data, list):
         # the bulk API always requires a list
@@ -122,6 +123,7 @@ def _apply_api_filter(api, eval_context):
     if not api_filter:
         return False
 
+    # root_doc here is the all data available as in corehq.motech.generic_inbound.utils.get_evaluation_context
     if not api_filter(eval_context.root_doc, eval_context):
         raise GenericInboundRequestFiltered()
 

corehq/motech/generic_inbound/backend/hl7.py

@@ -29,8 +29,8 @@ def get_basic_error_response(cls, request_id, status_code, message):
             content_type=HL7_CONTENT_TYPE
         )
 
-    def __init__(self, api_model, request_data):
-        super().__init__(api_model, request_data)
+    def __init__(self, configurable_api, request_data):
+        super().__init__(configurable_api, request_data)
         self.hl7_message = None
 
     def _get_body_for_eval_context(self):

corehq/motech/generic_inbound/models.py

@@ -31,10 +31,15 @@ class ConfigurableAPI(models.Model):
     updated_on = models.DateTimeField(auto_now=True)
     name = models.CharField(max_length=255)
     description = models.TextField(blank=True, null=True)
+    # UCR Expression to filter incoming/outgoing requests
+    # before they are processed/sent
     filter_expression = models.ForeignKey(
         UCRExpression, on_delete=models.PROTECT, related_name="api_filter", null=True, blank=True)
+    # UCR Expression to transform the data to be processed/sent
+    # from the original data received/to be sent
     transform_expression = models.ForeignKey(
         UCRExpression, on_delete=models.PROTECT, related_name="api_expression")
+    # type of data backend like JSON, HL7 etc
     backend = models.CharField(max_length=100, default=ApiBackendOptions.json)
 
     objects = AuditingManager()

corehq/motech/generic_inbound/templates/generic_inbound/api_edit.html

@@ -13,7 +13,7 @@
 {% registerurl 'ucr_expressions' domain %}
 <div id="edit-api">
   <div class="mb-3">
-    <pre id="api_url">{{ api_model.absolute_url }}</pre>
+    <pre id="api_url">{{ configurable_api.absolute_url }}</pre>
     <a class="btn btn-outline-primary" onclick="copyData('api_url')">
       <i class="fa fa-copy"></i> {% trans "Copy URL" %}
     </a>
@@ -37,7 +37,7 @@
           <tr>
             <td>
               <input type="hidden" data-bind="value: id, attr: {name: `validations-${$index()}-id`}">
-              <input type="hidden" value="{{ api_model.id }}" data-bind="attr: {name: `validations-${$index()}-api`}">
+              <input type="hidden" value="{{ configurable_api.id }}" data-bind="attr: {name: `validations-${$index()}-api`}">
               <input type="text" maxlength="64" class="textinput textInput form-control" required data-bind="
                 value: name, attr: {name: `validations-${$index()}-name`}, css: {'is-invalid': nameError}">
               <span data-bind="visible: nameError, css: {'invalid-feedback': nameError}">

corehq/motech/generic_inbound/tests/test_api_processing.py

@@ -36,50 +36,80 @@ class TestGenericInboundAPI(SimpleTestCase):
     domain_name = 'ucr-api-test'
 
     def test_spec_error(self):
-        api_model = ConfigurableAPI(
+        configurable_api = ConfigurableAPI(
             domain=self.domain_name,
             transform_expression=UCRExpression(definition={})
         )
         # mock 'get_validations' to prevent reverse foreign key lookup on unsaved obj
-        api_model.get_validations = lambda: []
+        configurable_api.get_validations = lambda: []
         user = MockUser()
-        context = get_evaluation_context(user, 'post', {}, {}, {})
+        context = get_evaluation_context(
+            restore_user=user,
+            method='post',
+            query={},
+            headers={},
+            body={}
+        )
         with self.assertRaises(BadSpecError):
-            _execute_generic_api(self.domain_name, user, "device_id", context, api_model)
+            _execute_generic_api(self.domain_name, user, "device_id", context, configurable_api)
 
     def test_no_filter(self):
-        api_model = ConfigurableAPI(
+        configurable_api = ConfigurableAPI(
             domain=self.domain_name,
             transform_expression=UCRExpression(definition={
                 'type': 'dict', 'properties': {'case_type': 'patient'}
             }),
         )
         user = MockUser()
-        context = get_evaluation_context(user, 'post', {}, {}, {"resource": {"type": "patient"}})
-        self.assertFalse(_apply_api_filter(api_model, context))
+        context = get_evaluation_context(
+            restore_user=user,
+            method='post',
+            query={},
+            headers={},
+            body={"resource": {"type": "patient"}}
+        )
+        self.assertFalse(_apply_api_filter(configurable_api, context))
 
     def test_filter_pass(self):
-        api_model = _get_api_with_filter(self.domain_name)
+        configurable_api = _get_api_with_filter(self.domain_name)
         user = MockUser()
-        context = get_evaluation_context(user, 'post', {}, {}, {"resource": {"type": "patient"}})
-        self.assertTrue(_apply_api_filter(api_model, context))
+        context = get_evaluation_context(
+            restore_user=user,
+            method='post',
+            query={},
+            headers={},
+            body={"resource": {"type": "patient"}}
+        )
+        self.assertTrue(_apply_api_filter(configurable_api, context))
 
     def test_filter_fail(self):
-        api_model = _get_api_with_filter(self.domain_name)
+        configurable_api = _get_api_with_filter(self.domain_name)
         user = MockUser()
-        context = get_evaluation_context(user, 'post', {}, {}, {"resource": {"type": "client"}})
+        context = get_evaluation_context(
+            restore_user=user,
+            method='post',
+            query={},
+            headers={},
+            body={"resource": {"type": "client"}}
+        )
         with self.assertRaises(GenericInboundRequestFiltered):
-            _apply_api_filter(api_model, context)
+            _apply_api_filter(configurable_api, context)
 
     def test_validation_pass(self):
-        api_model = _get_api_with_validation(self.domain_name)
+        configurable_api = _get_api_with_validation(self.domain_name)
         user = MockUser()
-        context = get_evaluation_context(user, 'post', {}, {}, {"resource": {"type": "patient"}})
-        self.assertTrue(_validate_api_request(api_model, context))
+        context = get_evaluation_context(
+            restore_user=user,
+            method='post',
+            query={},
+            headers={},
+            body={"resource": {"type": "patient"}}
+        )
+        self.assertTrue(_validate_api_request(configurable_api, context))
 
     def test_validation_errors(self):
-        api_model = _get_api_with_validation(self.domain_name)
-        validations = api_model.get_validations()
+        configurable_api = _get_api_with_validation(self.domain_name)
+        validations = configurable_api.get_validations()
         client_validation_expression = UCRExpression(
             expression_type=UCR_NAMED_FILTER,
             definition={
@@ -89,16 +119,22 @@ def test_validation_errors(self):
                 "property_value": "client"
             })
         validations.append(ConfigurableApiValidation(
-            api=api_model,
+            api=configurable_api,
             name="is also patient",
             message="must be patient again",
             expression=client_validation_expression,
         ))
         user = MockUser()
         # 1st validation should fail, 2nd should succeed
-        context = get_evaluation_context(user, 'post', {}, {}, {"resource": {"type": "employee"}})
+        context = get_evaluation_context(
+            restore_user=user,
+            method='post',
+            query={},
+            headers={},
+            body={"resource": {"type": "employee"}}
+        )
         with self.assertRaises(GenericInboundValidationError) as cm:
-            _execute_generic_api(self.domain_name, user, "device_id", context, api_model)
+            _execute_generic_api(self.domain_name, user, "device_id", context, configurable_api)
 
         self.assertEqual(cm.exception.errors, [
             {"name": "is patient", "message": "must be patient"},
@@ -132,43 +168,44 @@ def setUpTestData(cls):
         )
 
     def test_named_filter(self):
-        api_model = ConfigurableAPI(
+        configurable_api = ConfigurableAPI(
             domain=self.domain_name,
             filter_expression=UCRExpression(
                 expression_type=UCR_NAMED_FILTER,
                 definition={"type": "named", "name": "patient_case_filter"}
             ),
         )
         # this also tests that an exception isn't raised
-        self.assertIsNotNone(api_model.parsed_filter)
+        self.assertIsNotNone(configurable_api.parsed_filter)
 
     def test_named_expression(self):
-        api_model = ConfigurableAPI(
+        configurable_api = ConfigurableAPI(
             domain=self.domain_name,
             transform_expression=UCRExpression(
                 expression_type=UCR_NAMED_EXPRESSION,
                 definition={"type": "named", "name": "patient_case_create"}
             ),
         )
         # this also tests that an exception isn't raised
-        self.assertIsNotNone(api_model.parsed_expression)
+        self.assertIsNotNone(configurable_api.parsed_expression)
 
     def test_named_expression_in_validation(self):
         validation_expression = UCRExpression.objects.create(
             expression_type=UCR_NAMED_FILTER,
             definition={"type": "named", "name": "patient_case_filter"}
         )
 
-        api_model = ConfigurableAPI.objects.create(domain=self.domain_name, transform_expression=self.expression)
+        configurable_api = ConfigurableAPI.objects.create(domain=self.domain_name,
+                                                          transform_expression=self.expression)
         ConfigurableApiValidation.objects.create(
-            api=api_model, name="is patient", message="must be patient", expression=validation_expression
+            api=configurable_api, name="is patient", message="must be patient", expression=validation_expression
         )
         # this also tests that an exception isn't raised
-        self.assertIsNotNone(api_model.get_validations()[0].parsed_expression)
+        self.assertIsNotNone(configurable_api.get_validations()[0].parsed_expression)
 
 
 def _get_api_with_validation(domain_name, expression=None):
-    api_model = ConfigurableAPI(
+    configurable_api = ConfigurableAPI(
         domain=domain_name,
         transform_expression=UCRExpression(definition={
             'type': 'dict', 'properties': {'case_type': 'patient'}
@@ -184,15 +221,15 @@ def _get_api_with_validation(domain_name, expression=None):
         })
     validations = [
         ConfigurableApiValidation(
-            api=api_model,
+            api=configurable_api,
             name="is patient",
             message="must be patient",
             expression=validation_expression,
         )
     ]
     # mock 'get_validations'
-    api_model.get_validations = lambda: validations
-    return api_model
+    configurable_api.get_validations = lambda: validations
+    return configurable_api
 
 
 def _get_api_with_filter(domain_name):
@@ -204,11 +241,11 @@ def _get_api_with_filter(domain_name):
             "operator": "eq",
             "property_value": "patient"
         })
-    api_model = ConfigurableAPI(
+    configurable_api = ConfigurableAPI(
         domain=domain_name,
         filter_expression=filter_expression,
         transform_expression=UCRExpression(definition={
             'type': 'dict', 'properties': {'case_type': 'patient'}
         }),
     )
-    return api_model
+    return configurable_api

corehq/motech/generic_inbound/utils.py

@@ -1,11 +1,10 @@
-import json
 import uuid
 from base64 import urlsafe_b64encode
 from functools import cached_property
 
 from django.conf import settings
 from django.db import transaction
-from django.http import QueryDict, HttpResponse, JsonResponse
+from django.http import QueryDict, HttpResponse
 from django.utils.translation import gettext as _
 
 import attr
@@ -157,9 +156,9 @@ def get_request_data():
         make_processing_attempt(response, request_log, is_retry=True)
 
 
-def process_api_request(api_model, request_id, get_request_data):
+def process_api_request(configurable_api, request_id, get_request_data):
     try:
-        backend_cls = api_model.backend_class
+        backend_cls = configurable_api.backend_class
     except GenericInboundApiError as e:
         response = ApiResponse(status=500, internal_response={'error': str(e)})
     else:
@@ -168,7 +167,7 @@ def process_api_request(api_model, request_id, get_request_data):
         except GenericInboundUserError as e:
             response = backend_cls.get_basic_error_response(request_id, 400, str(e))
         else:
-            response = backend_cls(api_model, request_data).run()
+            response = backend_cls(configurable_api, request_data).run()
     return response