From 06805fef36aed494d2729a132a702b25e997980b Mon Sep 17 00:00:00 2001 From: Matt Griswold Date: Wed, 5 Feb 2020 21:26:21 -0600 Subject: [PATCH] Support 202001 (#641) * remove warning for missing information for fields: aka, looking glass server url, route server url (#616) remove warning for missing information for fields: ipv4 prefixes, ipv6 prefixes if either of them is set (#616) * Use autocomplete fields in the admincom controlpanel to speed up loading times (#597) * Better error reporting for POSTs and PUTs (#610) * Add operation to API to look for covering prefixes given an IP (#25) --- peeringdb_server/admin.py | 62 ++++++++++++++++++ .../management/commands/pdb_api_test.py | 65 ++++++++++++++++++- peeringdb_server/models.py | 20 ++++++ peeringdb_server/renderers.py | 3 +- peeringdb_server/rest.py | 65 +++++++++++++++++-- peeringdb_server/serializers.py | 5 +- peeringdb_server/static/peeringdb.js | 29 +++++++++ peeringdb_server/templates/site/view.html | 1 + peeringdb_server/views.py | 9 ++- 9 files changed, 247 insertions(+), 12 deletions(-) diff --git a/peeringdb_server/admin.py b/peeringdb_server/admin.py index 42d90436..c29f0f00 100644 --- a/peeringdb_server/admin.py +++ b/peeringdb_server/admin.py @@ -440,6 +440,11 @@ class InternetExchangeFacilityInline(SanitizedAdmin, admin.TabularInline): raw_id_fields = ("ix", "facility") form = StatusForm + autocomplete_lookup_fields = { + "fk": ["facility",], + } + + class NetworkContactInline(SanitizedAdmin, admin.TabularInline): model = NetworkContact @@ -455,6 +460,12 @@ class NetworkFacilityInline(SanitizedAdmin, admin.TabularInline): "network", ) form = StatusForm + raw_id_fields = ("facility",) + autocomplete_lookup_fields = { + "fk": ["facility",], + } + + class NetworkIXLanValidationMixin(object): @@ -549,6 +560,12 @@ class InternetExchangeAdmin(ModelAdminWithVQCtrl, SoftDeleteAdmin): inlines = (InternetExchangeFacilityInline, IXLanInline) form = InternetExchangeAdminForm + raw_id_fields = ("org",) + autocomplete_lookup_fields = { + "fk": ["org"], + } + + def ixf_import_history(self, obj): return mark_safe( '{}'.format( @@ -575,6 +592,12 @@ class IXLanAdmin(SoftDeleteAdmin): readonly_fields = ("id",) inlines = (IXLanPrefixInline, NetworkInternetExchangeInline) form = IXLanAdminForm + raw_id_fields = ("ix",) + autocomplete_lookup_fields = { + "fk": ["ix",], + } + + class IXLanIXFMemberImportLogEntryInline(admin.TabularInline): @@ -875,6 +898,13 @@ class FacilityAdmin(ModelAdminWithVQCtrl, SoftDeleteAdmin): list_filter = (StatusFilter,) search_fields = ("name",) readonly_fields = ("id", "nsp_namespace") + + raw_id_fields = ("org",) + autocomplete_lookup_fields = { + "fk": ["org"], + } + + form = FacilityAdminForm inlines = ( InternetExchangeFacilityInline, @@ -909,6 +939,13 @@ class NetworkAdmin(ModelAdminWithVQCtrl, SoftDeleteAdmin): NetworkInternetExchangeInline, ) + raw_id_fields = ("org",) + autocomplete_lookup_fields = { + "fk": ["org",], + } + + + class InternetExchangeFacilityAdmin(SoftDeleteAdmin): list_display = ("id", "ix", "facility", "status", "created", "updated") @@ -917,6 +954,12 @@ class InternetExchangeFacilityAdmin(SoftDeleteAdmin): list_filter = (StatusFilter,) form = StatusForm + raw_id_fields = ("ix", "facility") + autocomplete_lookup_fields = { + "fk": ["ix", "facility"], + } + + class IXLanPrefixAdmin(SoftDeleteAdmin): list_display = ("id", "prefix", "ixlan", "ix", "status", "created", "updated") @@ -955,6 +998,12 @@ class NetworkIXLanAdmin(SoftDeleteAdmin): list_filter = (StatusFilter,) form = StatusForm + raw_id_fields = ("network",) + autocomplete_lookup_fields = { + "fk": ["network",], + } + + def ix(self, obj): return obj.ixlan.ix @@ -979,6 +1028,12 @@ class NetworkContactAdmin(SoftDeleteAdmin): list_filter = (StatusFilter,) form = StatusForm + raw_id_fields = ("network",) + autocomplete_lookup_fields = { + "fk": ["network",], + } + + def net(self, obj): return "{} (AS{})".format(obj.network.name, obj.network.asn) @@ -990,6 +1045,13 @@ class NetworkFacilityAdmin(SoftDeleteAdmin): list_filter = (StatusFilter,) form = StatusForm + raw_id_fields = ("network", "facility") + autocomplete_lookup_fields = { + "fk": ["network", "facility"], + } + + + def net(self, obj): return "{} (AS{})".format(obj.network.name, obj.network.asn) diff --git a/peeringdb_server/management/commands/pdb_api_test.py b/peeringdb_server/management/commands/pdb_api_test.py index 79ea42d8..b8f95971 100644 --- a/peeringdb_server/management/commands/pdb_api_test.py +++ b/peeringdb_server/management/commands/pdb_api_test.py @@ -11,6 +11,7 @@ import random import re import time import datetime +import json from twentyc.rpc import ( RestClient, @@ -30,6 +31,7 @@ from django.conf import settings from django.db.utils import IntegrityError from rest_framework import serializers +from rest_framework.test import APIRequestFactory from peeringdb_server.models import ( REFTAG_MAP, @@ -49,6 +51,7 @@ from peeringdb_server.models import ( from peeringdb_server.serializers import REFTAG_MAP as REFTAG_MAP_SLZ from peeringdb_server import inet, settings as pdb_settings +from peeringdb_server.rest import NetworkViewSet START_TIMESTAMP = time.time() @@ -541,7 +544,7 @@ class TestJSON(unittest.TestCase): for k, v in list(test_failures["invalid"].items()): self.assertIn(k, list(r.keys())) - assert "400 Unknown" in str(excinfo.value) + assert "400 Bad Request" in str(excinfo.value) # we test fail because of parent entity status if "status" in test_failures: @@ -612,7 +615,7 @@ class TestJSON(unittest.TestCase): with pytest.raises(InvalidRequestException) as excinfo: db.update(typ, **data_invalid) - assert "400 Unknown" in str(excinfo.value) + assert "400 Bad Request" in str(excinfo.value) # we test fail because of permissions if "perms" in test_failures: @@ -620,6 +623,10 @@ class TestJSON(unittest.TestCase): for k, v in list(test_failures["perms"].items()): data_perms[k] = v + # if data is empty set something so we dont + # trigger the empty data error + data_perms["_dummy_"] = 1 + with pytest.raises(PermissionDeniedException): db.update(typ, **data_perms) @@ -2061,6 +2068,18 @@ class TestJSON(unittest.TestCase): ########################################################################## + def test_guest_005_list_filter_ixpfx_whereis(self): + ixpfx = SHARED["ixpfx_r_ok"] + + ipaddr = "{}".format(ixpfx.prefix[0]) + + data = self.db_guest.all("ixpfx", whereis=ipaddr) + + assert len(data) == 1 + assert data[0]["id"] == ixpfx.id + + ########################################################################## + def test_guest_005_list_filter_ix_related(self): self.assert_list_filter_related("ix", "ixlan") self.assert_list_filter_related("ix", "ixfac") @@ -3102,6 +3121,48 @@ class TestJSON(unittest.TestCase): fac.refresh_from_db() self.assertEqual(fac.geocode_status, True) + def test_z_misc_001_api_errors(self): + """ + Test empty POST, PUT data error response + Test parse error POST, PUT data error response + """ + for reftag in list(REFTAG_MAP.keys()): + self._test_z_misc_001_api_errors(reftag, "post", "create") + self._test_z_misc_001_api_errors(reftag, "put", "update") + + def _test_z_misc_001_api_errors(self, reftag, method, action): + factory = APIRequestFactory() + url = "/{}/".format(reftag) + view_action = {method: action} + view = NetworkViewSet.as_view(view_action) + fn = getattr(factory, method) + + ERR_PARSE = "Data supplied with the {} request could not be parsed: JSON parse error - Expecting value: line 1 column 1 (char 0)".format( + method.upper() + ) + ERR_MISSING = "No data was supplied with the {} request".format(method.upper()) + + # test posting invalid json error + + request = fn(url, "in{valid json", content_type="application/json") + response = view(request) + response.render() + assert json.loads(response.content)["meta"]["error"] == ERR_PARSE + + # test posting empty json error + + request = fn("/net/", "{}", content_type="application/json") + response = view(request) + response.render() + assert json.loads(response.content)["meta"]["error"] == ERR_MISSING + + # test posting empty json error + + request = fn("/net/", "", content_type="application/json") + response = view(request) + response.render() + assert json.loads(response.content)["meta"]["error"] == ERR_MISSING + class Command(BaseCommand): help = "This runs the api test harness. All write ops are performed under an organization specifically made for testing, so running to against a prod environment should be fine in theory." diff --git a/peeringdb_server/models.py b/peeringdb_server/models.py index 7b6bf7e3..16e90fb0 100644 --- a/peeringdb_server/models.py +++ b/peeringdb_server/models.py @@ -1999,6 +1999,26 @@ class IXLanPrefix(pdb_models.IXLanPrefixBase): filt = make_relation_filter("ixlan__%s" % field, filt, value) return qset.filter(**filt) + @classmethod + def whereis_ip(cls, ipaddr, qset=None): + """ + Filter queryset of ixpfx objects where the prefix contains + the supplied ipaddress + """ + + if not qset: + qset = cls.handleref.undeleted() + + ids = [] + + ipaddr = ipaddress.ip_address(ipaddr) + + for ixpfx in qset: + if ipaddr in ixpfx.prefix: + ids.append(ixpfx.id) + + return qset.filter(id__in=ids) + @property def nsp_namespace(self): """ diff --git a/peeringdb_server/renderers.py b/peeringdb_server/renderers.py index 1c9c49f1..a8984f6b 100644 --- a/peeringdb_server/renderers.py +++ b/peeringdb_server/renderers.py @@ -87,7 +87,8 @@ class MetaJSONRenderer(MungeRenderer): result["data"] = [] elif res.status_code < 500: - meta["error"] = data.pop("detail", "Unknown") + meta["error"] = data.pop("detail", res.reason_phrase) + result.update(**data) result["meta"] = meta diff --git a/peeringdb_server/rest.py b/peeringdb_server/rest.py index 8fd7ddc7..bd085b5f 100644 --- a/peeringdb_server/rest.py +++ b/peeringdb_server/rest.py @@ -11,7 +11,7 @@ import unidecode from rest_framework import routers, serializers, status, viewsets from rest_framework.response import Response from rest_framework.views import exception_handler -from rest_framework.exceptions import ValidationError as RestValidationError +from rest_framework.exceptions import ValidationError as RestValidationError, ParseError from django.apps import apps from django.conf import settings @@ -30,6 +30,39 @@ from peeringdb_server.api_cache import CacheRedirect, APICacheLoader import django_namespace_perms.util as nsp from django_namespace_perms.exceptions import * + +class DataException(ValueError): + pass + + +class DataMissingException(DataException): + + """ + Will be raised when the json data sent with a POST, PUT or PATCH + request is missing + """ + + def __init__(self, method): + super(DataMissingException, self).__init__( + "No data was supplied with the {} request".format(method) + ) + + +class DataParseException(DataException): + + """ + Will be raised when the json data sent with a POST, PUT or PATCH + request could not be parsed + """ + + def __init__(self, method, exc): + super(DataParseException, self).__init__( + "Data supplied with the {} request could not be parsed: {}".format( + method, exc + ) + ) + + ############################################################################### @@ -261,11 +294,11 @@ class ModelViewSet(viewsets.ModelViewSet): qset, **self.request.query_params ) except ValidationError as inst: - raise RestValidationError({"detail": str(inst[0])}) + raise RestValidationError({"detail": str(inst)}) except ValueError as inst: - raise RestValidationError({"detail": str(inst[0])}) + raise RestValidationError({"detail": str(inst)}) except TypeError as inst: - raise RestValidationError({"detail": str(inst[0])}) + raise RestValidationError({"detail": str(inst)}) except FieldError as inst: raise RestValidationError({"detail": "Invalid query"}) @@ -474,19 +507,40 @@ class ModelViewSet(viewsets.ModelViewSet): return r + def require_data(self, request): + """ + Test that the request contains data in it's body that + can be parsed to the required format (json) and is not + empty + + Will raise DataParseException error if request payload could + not be parsed + + Will raise DataMissingException error if request payload is + missing or was parsed to an empty object + """ + try: + request.data + except ParseError as exc: + raise DataParseException(request.method, exc) + + if not request.data: + raise DataMissingException(request.method) + @client_check() def create(self, request, *args, **kwargs): """ Create object """ try: + self.require_data(request) with reversion.create_revision(): if request.user: reversion.set_user(request.user) return super(ModelViewSet, self).create(request, *args, **kwargs) except PermissionDenied as inst: return Response(status=status.HTTP_403_FORBIDDEN) - except ParentStatusException as inst: + except (ParentStatusException, DataException) as inst: return Response( status=status.HTTP_400_BAD_REQUEST, data={"detail": str(inst)} ) @@ -499,6 +553,7 @@ class ModelViewSet(viewsets.ModelViewSet): Update object """ try: + self.require_data(request) with reversion.create_revision(): if request.user: reversion.set_user(request.user) diff --git a/peeringdb_server/serializers.py b/peeringdb_server/serializers.py index 7c3f660c..cee50a5d 100644 --- a/peeringdb_server/serializers.py +++ b/peeringdb_server/serializers.py @@ -1716,7 +1716,7 @@ class IXLanPrefixSerializer(ModelSerializer): @classmethod def prepare_query(cls, qset, **kwargs): - filters = get_relation_filters(["ix_id", "ix"], cls, **kwargs) + filters = get_relation_filters(["ix_id", "ix", "whereis"], cls, **kwargs) for field, e in list(filters.items()): for valid in ["ix"]: if validate_relation_filter_field(field, valid): @@ -1724,6 +1724,9 @@ class IXLanPrefixSerializer(ModelSerializer): qset = fn(qset=qset, field=field, **e) break + if field == "whereis": + qset = cls.Meta.model.whereis_ip(e["value"], qset=qset) + return qset.select_related("ixlan", "ixlan__ix"), filters def has_create_perms(self, user, data): diff --git a/peeringdb_server/static/peeringdb.js b/peeringdb_server/static/peeringdb.js index bb4cd46d..002e8780 100644 --- a/peeringdb_server/static/peeringdb.js +++ b/peeringdb_server/static/peeringdb.js @@ -162,8 +162,37 @@ PeeringDB = { var value = $(this).html().trim(); var name = $(this).data("edit-name"); var field = $(this).prev('.view_field'); + var group = field.data("notify-incomplete-group") + if(!field.length) field = $(this).parent().prev('.view_field'); + + // if field is part of a notify-incomplete-group + // it means we don't want to show a warning as long + // as one of the members of the group has it's value set + + if(group && (value == "" || value == "0")) { + var other, i, others, _value; + + // get other members of the group + + others = $('[data-notify-incomplete-group="'+group+'"]') + + for(i = 0; i < others.length; i++) { + other = $(others[i]).next('.view_value') + _value = other.html().trim() + + // other group member's value is set, use that value + // to toggle that we do not want to show a notification + // warning for this field + + if(_value != "" && _value != "0") { + value = _value + break; + } + } + } + var check = (field.find('.incomplete').length == 1); if(check && (value == "" || value == "0")) { status.incomplete = true; diff --git a/peeringdb_server/templates/site/view.html b/peeringdb_server/templates/site/view.html index 8db1d231..15a091f8 100644 --- a/peeringdb_server/templates/site/view.html +++ b/peeringdb_server/templates/site/view.html @@ -96,6 +96,7 @@ {% if row.help_text %} data-toggle="tooltip" data-placement="top" + {% if row.notify_incomplete_group %}data-notify-incomplete-group="{{ row.notify_incomplete_group }}"{% endif %} title="{{ row.help_text }}" {% endif %}>{% if row.notify_incomplete %}{% endif %} {{ row.label }} {% if row.help_text %}{% endif %} diff --git a/peeringdb_server/views.py b/peeringdb_server/views.py index 8c07f87e..a396af0b 100644 --- a/peeringdb_server/views.py +++ b/peeringdb_server/views.py @@ -170,6 +170,7 @@ def beta_sync_dt(): return dt.replace(hour=0, minute=0, second=0) + def update_env_beta_sync_dt(env): if settings.RELEASE_ENV == "beta": env.update(beta_sync_dt=beta_sync_dt()) @@ -1406,7 +1407,7 @@ def view_network(request, id): { "name": "aka", "label": _("Also Known As"), - "notify_incomplete": True, + "notify_incomplete": False, "value": network_d.get("aka", dismiss), }, { @@ -1433,14 +1434,14 @@ def view_network(request, id): "name": "route_server", "type": "url", "label": _("Route Server URL"), - "notify_incomplete": True, + "notify_incomplete": False, "value": network_d.get("route_server", dismiss), }, { "name": "looking_glass", "type": "url", "label": _("Looking Glass URL"), - "notify_incomplete": True, + "notify_incomplete": False, "value": network_d.get("looking_glass", dismiss), }, { @@ -1458,6 +1459,7 @@ def view_network(request, id): "type": "number", "help_text": field_help(Network, "info_prefixes4"), "notify_incomplete": True, + "notify_incomplete_group": "prefixes", "value": int(network_d.get("info_prefixes4") or 0), }, { @@ -1466,6 +1468,7 @@ def view_network(request, id): "type": "number", "help_text": field_help(Network, "info_prefixes6"), "notify_incomplete": True, + "notify_incomplete_group": "prefixes", "value": int(network_d.get("info_prefixes6") or 0), }, {