Merge pull request #29 from maykinmedia/feature/28-local-base-urls

Feature/28 local base urls

README.rst

@@ -125,6 +125,20 @@ or override the loader on a per-field basis:
             loader=RequestsLoader()
         )
 
+Local and remote urls
+---------------------
+
+If several services are hosted within the same domain, it could be tricky to separate
+local and remote urls. In this case an additional setting ``LOOSE_FK_LOCAL_BASE_URLS``can be used
+to define an explicit list of allowed prefixes for local urls.
+
+.. code-block:: python
+
+    LOOSE_FK_LOCAL_BASE_URLS = [
+        "https://api.example.nl/ozgv-t/zaken/",
+        "https://api.example.nl/ozgv-t/catalogi/",
+    ]
+
 
 .. |build-status| image:: https://github.com/maykinmedia/django-loose-fk/workflows/Run%20CI/badge.svg
     :alt: Build status

django_loose_fk/constraints.py

@@ -1,5 +1,6 @@
 from django.db import models
 from django.db.models.constraints import BaseConstraint
+from django.db.utils import DEFAULT_DB_ALIAS
 
 
 class FkOrURLFieldConstraint(BaseConstraint):
@@ -35,7 +36,7 @@ def deconstruct(self):
         )
         return (path, args, kwargs)
 
-    def _get_check_constraint(self, model, schema_editor):
+    def _get_check_constraint(self, model, schema_editor=None):
         """
         Return the underlying check constraint generated.
         """
@@ -66,6 +67,10 @@ def remove_sql(self, model, schema_editor):
         check_constraint = self._get_check_constraint(model, schema_editor)
         return check_constraint.remove_sql(model, schema_editor)
 
+    def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS):
+        check_constraint = self._get_check_constraint(model)
+        return check_constraint.validate(model, instance, exclude, using)
+
     def __repr__(self):
         return "<%s: field=%r>" % (self.__class__.__name__, self.name)
 

django_loose_fk/drf.py

@@ -3,6 +3,7 @@
 
 Provides a custom field.
 """
+
 import logging
 from dataclasses import dataclass
 from typing import Tuple, Union
@@ -20,7 +21,7 @@
 
 from .fields import FkOrURLField, InstanceOrUrl
 from .loaders import FetchError, FetchJsonError
-from .utils import get_resource_for_path
+from .utils import get_resource_for_path, is_local
 
 logger = logging.getLogger(__name__)
 
@@ -45,8 +46,8 @@ class Resolver:
 
     def resolve(self, host: str, url: str) -> models.Model:
         parsed = urlparse(url)
-        is_local = parsed.netloc == host
-        return self.resolve_local(parsed) if is_local else self.resolve_remote(url)
+        local = is_local(host, url)
+        return self.resolve_local(parsed) if local else self.resolve_remote(url)
 
     def resolve_local(self, parsed: ParseResult) -> models.Model:
         return get_resource_for_path(parsed.path)

django_loose_fk/filters.py

@@ -1,8 +1,8 @@
 """
 Filter support for django-filter.
 """
+
 import logging
-from functools import reduce
 from urllib.parse import urlparse
 
 from django import forms
@@ -12,7 +12,7 @@
 from django_filters.filterset import FilterSet, remote_queryset as _remote_queryset
 
 from .fields import FkOrURLField
-from .utils import get_resource_for_path, get_subclasses
+from .utils import get_resource_for_path, get_subclasses, is_local
 
 logger = logging.getLogger(__name__)
 
@@ -74,7 +74,7 @@ def get_filters(self, model_field, parsed_values, host) -> dict:
 
         filters = {}
         for value in parsed_values:
-            local = value.netloc == host
+            local = is_local(host, value.geturl())
             if local:
                 local_object = get_resource_for_path(value.path)
                 if self.instance_path:

django_loose_fk/inspectors/query.py

@@ -4,6 +4,7 @@
 Requires drf-yasg, which can be installed as the [openapi] optional group
 dependency.
 """
+
 from django_filters.rest_framework.backends import DjangoFilterBackend
 from drf_yasg import openapi
 

django_loose_fk/loaders.py

@@ -46,6 +46,7 @@ def is_local_url(self, url: str) -> bool:
         """
         allowed_hosts = settings.ALLOWED_HOSTS
         parsed = urlparse(url)
+        local_base_urls = getattr(settings, "LOOSE_FK_LOCAL_BASE_URLS", [])
 
         if any(pattern == "*" for pattern in allowed_hosts):
             warnings.warn(
@@ -54,6 +55,10 @@ def is_local_url(self, url: str) -> bool:
                 "break django-loose-fk's behaviour. You should use an explicit list.",
                 RuntimeWarning,
             )
+            return validate_host(parsed.netloc, allowed_hosts)
+
+        if local_base_urls:
+            return any(url.startswith(base_url) for base_url in local_base_urls)
 
         return validate_host(parsed.netloc, allowed_hosts)
 

django_loose_fk/lookups.py

@@ -116,6 +116,14 @@ def get_prep_lookup(self):
         if self.rhs_is_direct_value():
             # If we get here, we are dealing with single-column relations.
             self.rhs = [get_normalized_value(val)[0] for val in self.rhs]
+
+        else:
+            # we're dealing with something that can be expressed as SQL -> it's local only!
+            target = self.lhs.target
+            db_table = target.model._meta.db_table
+            fk_lhs = target._fk_field.get_col(db_table)
+            target_field = fk_lhs.target.target_field.name
+            self.rhs.set_values([target_field])
         return super().get_prep_lookup()
 
     def as_sql(self, compiler, connection):

django_loose_fk/utils.py

@@ -1,3 +1,5 @@
+from urllib.parse import urlparse
+
 from django.conf import settings
 from django.db import models
 from django.http import HttpRequest
@@ -7,6 +9,21 @@
 from rest_framework.request import Request
 
 
+def is_local(host: str, url: str) -> bool:
+    """
+    Define if the url is local or external based on LOOSE_FK_LOCAL_BASE_URLS
+    setting or a host
+    """
+    parsed = urlparse(url)
+    local_base_urls = getattr(settings, "LOOSE_FK_LOCAL_BASE_URLS", [])
+    # if local base urls are defined - use them
+    if local_base_urls:
+        return any(url.startswith(base_url) for base_url in local_base_urls)
+
+        # otherwise use hostname
+    return parsed.netloc == host
+
+
 def get_viewset_for_path(path: str) -> viewsets.ViewSet:
     """
     Look up which viewset matches a path.

tests/test_api.py

@@ -1,6 +1,7 @@
 """
 Test the API interface to handle local/remote references.
 """
+
 from unittest.mock import patch
 
 from django.test import override_settings
@@ -191,3 +192,45 @@ def test_filter_zaaktype_local_fk(api_client):
 
     assert len(response.data) == 1
     assert response.data[0]["url"] == f"http://testserver.com{zaak_url}"
+
+
+@patch("django_loose_fk.utils.get_script_prefix", return_value="/subpath/")
+@override_settings(
+    ALLOWED_HOSTS=["testserver.com"],
+    LOOSE_FK_LOCAL_BASE_URLS=["http://testserver.com/subpath/zaaktypes/"],
+)
+def test_write_local_url_with_local_base_urls(mock, api_client):
+    url = reverse("zaak-list")
+    zaaktype = ZaakType.objects.create(name="test")
+    zaaktype_url = reverse("zaaktype-detail", kwargs={"pk": zaaktype.pk})
+
+    data = {"name": "test", "zaaktype": f"http://testserver.com/subpath{zaaktype_url}"}
+
+    response = api_client.post(url, data, HTTP_HOST="testserver.com")
+
+    assert response.status_code == 201
+    zaak = Zaak.objects.get()
+    assert zaak.zaaktype == zaaktype
+
+
+@patch("django_loose_fk.utils.get_script_prefix", return_value="/subpath/")
+@override_settings(
+    ALLOWED_HOSTS=["testserver.com"],
+    LOOSE_FK_LOCAL_BASE_URLS=["http://testserver.com/subpath/zaaktypes/"],
+)
+def test_write_remote_url_with_local_base_urls(mock, api_client):
+    """
+    test that service with the same host and prefix can still be external
+    """
+    url = reverse("zaak-list")
+    zaaktype_url = "http://testserver.com/subpath/service/zaaktypes/10/"
+    data = {"name": "test", "zaaktype": zaaktype_url}
+
+    with requests_mock.Mocker() as m:
+        m.get(zaaktype_url, json={"url": zaaktype_url, "name": "test"})
+
+        response = api_client.post(url, data, HTTP_HOST="testserver.com")
+
+        assert response.status_code == 201
+        zaak = Zaak.objects.get()
+        assert zaak.zaaktype == zaaktype_url

tests/test_chain_loose_fk.py

@@ -1,6 +1,7 @@
 """
 Test the API interface to handle chained local/remote references.
 """
+
 import pytest
 import requests_mock
 from rest_framework.reverse import reverse

tests/test_compare.py

@@ -1,6 +1,7 @@
 """
 Test the API interface to handle comparing of loose-fk urls
 """
+
 import pytest
 import requests_mock
 

tests/test_is_local.py

@@ -0,0 +1,56 @@
+"""
+Test django_loose_fk.utils.is_local function with various
+combinations of settings
+"""
+
+from django.test import override_settings
+
+from django_loose_fk.utils import is_local
+
+
+@override_settings(
+    LOOSE_FK_LOCAL_BASE_URLS=["http://api.example.nl/ozgv-t/zaken"],
+)
+def test_with_setting_same_host_same_prefix():
+    assert (
+        is_local(host="api.example.nl", url="http://api.example.nl/ozgv-t/zaken/1")
+        is True
+    )
+
+
+@override_settings(
+    LOOSE_FK_LOCAL_BASE_URLS=["http://api.example.nl/ozgv-t/zaken"],
+)
+def test_with_setting_same_host_diff_prefix():
+    assert (
+        is_local(host="api.example.nl", url="http://api.example.nl/ozgv-t/documenten/1")
+        is False
+    )
+
+
+@override_settings(
+    LOOSE_FK_LOCAL_BASE_URLS=["http://otherapi.example.nl/ozgv-t/zaken"],
+)
+def test_with_setting_diff_host():
+    assert (
+        is_local(host="api.example.nl", url="http://otherapi.example.nl/ozgv-t/zaken/1")
+        is True
+    )
+
+
+def test_no_setting_same_host_with_prefix():
+    assert (
+        is_local(host="api.example.nl", url="http://api.example.nl/ozgv-t/zaken/1")
+        is True
+    )
+
+
+def test_no_setting_same_host_no_prefix():
+    assert is_local(host="api.example.nl", url="http://api.example.nl/zaken/1") is True
+
+
+def test_no_setting_diff_host():
+    assert (
+        is_local(host="api.example.nl", url="http://otherapi.example.nl/ozgv-t/zaken/1")
+        is False
+    )

tests/test_model_field_interface.py

@@ -1,6 +1,7 @@
 """
 Test that it's possibly to handle remote/local objects transparently.
 """
+
 import uuid
 
 import pytest

tests/test_querying.py

@@ -1,6 +1,7 @@
 """
 Test the ORM queries against the virtual field.
 """
+
 import pytest
 import requests_mock