Skip to content

Duplicate error msg fix for IP address field #9647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion rest_framework/utils/field_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,10 @@ def get_field_kwargs(field_name, model_field):
if isinstance(model_field, models.GenericIPAddressField):
validator_kwarg = [
validator for validator in validator_kwarg
if validator is not validators.validate_ipv46_address
if validator not in [validators.validate_ipv46_address, validators.validate_ipv6_address, validators.validate_ipv4_address]
]
kwargs['protocol'] = getattr(model_field, 'protocol', 'both')

# Our decimal validation is handled in the field code, not validator code.
if isinstance(model_field, models.DecimalField):
validator_kwarg = [
Expand Down
97 changes: 97 additions & 0 deletions tests/test_ipaddress_field_serializer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
from django.db import models
from django.test import TestCase

from rest_framework import serializers
from rest_framework.exceptions import ValidationError


# Define the model
class TestModel(models.Model):
address = models.GenericIPAddressField(protocol="both")

class Meta:
app_label = "main"


class TestSerializer(serializers.ModelSerializer):

class Meta:
model = TestModel
fields = "__all__"


# Define the serializer in setUp
class TestSerializerTestCase(TestCase):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected this test to be next to the existin ones:

class TestGenericIPAddressFieldValidation(TestCase):

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out the existing test cases @browniebroke , I did some changes and incorporated my test cases within the same class, and did some minor changes along with it.

def setUp(self):
"""Initialize serializer class."""
self.serializer_class = TestSerializer

def test_invalid_ipv4_for_ipv4_field(self):
"""Test that an invalid IPv4 raises only an IPv4-related error."""
TestModel._meta.get_field("address").protocol = "IPv4" # Set field to IPv4 only
invalid_data = {"address": "invalid-ip"}
serializer = self.serializer_class(data=invalid_data)

with self.assertRaises(ValidationError) as context:
serializer.is_valid(raise_exception=True)

self.assertEqual(
str(context.exception.detail["address"][0]),
"Enter a valid IPv4 address."
)

def test_invalid_ipv6_for_ipv6_field(self):
"""Test that an invalid IPv6 raises only an IPv6-related error."""
TestModel._meta.get_field("address").protocol = "IPv6" # Set field to IPv6 only
invalid_data = {"address": "invalid-ip"}
serializer = self.serializer_class(data=invalid_data)

with self.assertRaises(ValidationError) as context:
serializer.is_valid(raise_exception=True)

self.assertEqual(
str(context.exception.detail["address"][0]),
"Enter a valid IPv6 address."
)

def test_invalid_both_protocol(self):
"""Test that an invalid IP raises a combined error message when protocol is both."""
TestModel._meta.get_field("address").protocol = "both" # Allow both IPv4 & IPv6
invalid_data = {"address": "invalid-ip"}
serializer = self.serializer_class(data=invalid_data)

with self.assertRaises(ValidationError) as context:
serializer.is_valid(raise_exception=True)

self.assertEqual(
str(context.exception.detail["address"][0]),
"Enter a valid IPv4 or IPv6 address."
)

def test_valid_ipv4(self):
"""Test that a valid IPv4 passes validation."""
TestModel._meta.get_field("address").protocol = "IPv4"
valid_data = {"address": "192.168.1.1"}
serializer = self.serializer_class(data=valid_data)
self.assertTrue(serializer.is_valid())

def test_valid_ipv6(self):
"""Test that a valid IPv6 passes validation."""
TestModel._meta.get_field("address").protocol = "IPv6"
valid_data = {"address": "2001:db8::ff00:42:8329"}
serializer = self.serializer_class(data=valid_data)
self.assertTrue(serializer.is_valid())

def test_valid_ipv4_for_both_protocol(self):
"""Test that a valid IPv4 is accepted when protocol is 'both'."""
TestModel._meta.get_field("address").protocol = "both"
valid_data = {"address": "192.168.1.1"}
serializer = self.serializer_class(data=valid_data)
self.assertTrue(serializer.is_valid())

def test_valid_ipv6_for_both_protocol(self):
"""Test that a valid IPv6 is accepted when protocol is 'both'."""
TestModel._meta.get_field("address").protocol = "both"
valid_data = {"address": "2001:db8::ff00:42:8329"}
serializer = self.serializer_class(data=valid_data)
self.assertTrue(serializer.is_valid())
Loading