fix: improve test coverage and fix bugs along the way

src/json_agg/aggregates.py

@@ -11,6 +11,7 @@
 from django.db.models import Field
 from django.db.models import Func
 from django.db.models import JSONField
+from django.db.models import Q
 
 
 class JSONAggregateMixin(abc.ABC):
@@ -83,17 +84,36 @@ def __init__(
         }[connection.vendor]
         if vendor_func := kwargs.get(f"{connection.vendor}_func"):
             value_expression = Func(value_expression, function=vendor_func)
+        # key can't be NULL, so lets exclude it
+        not_null_key_filter = Q(**{f"{name_expression}__isnull": False})
+        filters = kwargs.pop("filter", None)
+        if not filters:
+            filters = not_null_key_filter
+        else:
+            filters = filters & not_null_key_filter
         super().__init__(
             name_expression,
             value_expression,
+            filter=filters,
             **kwargs,
         )
 
     def _convert_nested_value(self, value, converter):
         if not value:
-            return None
+            return {}
         return {k: converter(v) for k, v in value.items()}
 
+    @property
+    def convert_value(self) -> callable:
+        """Override BaseExpression.convert_value to handle json objects."""
+
+        def _converter(value, expression, connection):
+            if not value:
+                return "{}"
+            return value
+
+        return _converter
+
 
 class JSONArrayAgg(JSONAggregateMixin, Aggregate):
     """Aggregate as JSON array.
@@ -122,6 +142,6 @@ def __init__(self, expression: Any, **kwargs):
         super().__init__(expression, **kwargs)
 
     def _convert_nested_value(self, value, converter):
-        if not value:
-            return None
+        if not value:  # pragma: no cover
+            return []
         return [converter(v) for v in value]

tests/models.py

@@ -8,10 +8,10 @@
 class Post(models.Model):
     """Model representing a blog post."""
 
-    title = models.CharField(max_length=100)
+    title = models.CharField(max_length=100, null=True)
     year = models.IntegerField(default=datetime.now().year)
     updated_at = models.DateTimeField(null=True)
-    content = models.TextField()
+    content = models.TextField(null=True)
     metadata = models.JSONField(default=dict)
     author = models.ForeignKey(
         "tests.Author", related_name="posts", on_delete=models.CASCADE

tests/test_array_agg.py

@@ -12,6 +12,7 @@
 
 from json_agg import JSONArrayAgg
 from tests.models import Author
+from tests.models import Post
 from tests.post_factory import post_factory
 
 
@@ -21,7 +22,7 @@
 
 @pytest.mark.django_db
 def test_aggregate_integer(faker: Faker):
-    """Test JSONObjectAgg over a integer value (Post.year)."""
+    """Test JSONArrayAgg over a integer value (Post.year)."""
     expected_value_per_author_name = post_factory(
         faker,
         value_name="year",
@@ -37,7 +38,7 @@ def test_aggregate_integer(faker: Faker):
 
 @pytest.mark.django_db
 def test_aggregate_text(faker: Faker):
-    """Test JSONObjectAgg over a integer value (Post.content)."""
+    """Test JSONArrayAgg over a integer value (Post.content)."""
     expected_value_per_author_name = post_factory(
         faker,
         value_name="content",
@@ -53,7 +54,7 @@ def test_aggregate_text(faker: Faker):
 
 @pytest.mark.django_db
 def test_aggregate_datetime(faker: Faker, db_vendor: str):
-    """Test JSONObjectAgg over a datetime value (Post.updated_at)."""
+    """Test JSONArrayAgg over a datetime value (Post.updated_at)."""
     kw = {}
     if db_vendor == "postgresql":
         # enforce tz for postgresql only - sqlite don't support it.
@@ -78,7 +79,7 @@ def test_aggregate_datetime(faker: Faker, db_vendor: str):
 
 @pytest.mark.django_db
 def test_aggregate_json(faker: Faker):
-    """Test JSONObjectAgg over a json value (Post.metadata)."""
+    """Test JSONArrayAgg over a json value (Post.metadata)."""
     expected_value_per_author_name = post_factory(
         faker,
         value_name="metadata",
@@ -97,3 +98,75 @@ def test_aggregate_json(faker: Faker):
     result_as_dict = {author.name: author.json_array for author in queryset}
     diff = DeepDiff(result_as_dict, expected_value_per_author_name, ignore_order=True)
     assert diff == {}, diff
+
+
+def test_raise_value_error_invalid_nested_output_field():
+    """Ensure ValueError is raised if invalid type is used for nested_output_field."""
+    with pytest.raises(ValueError):
+        JSONArrayAgg("foo", nested_output_field=True)
+
+
+@pytest.mark.django_db
+def test_with_no_related_objects(faker: Faker):
+    """Test JSONArrayAgg aggregating when there are no related objects."""
+    author_name = faker.name()
+    Author.objects.create(name=author_name)
+
+    annotated_result = Author.objects.annotate(
+        json_array=JSONArrayAgg("posts__title")
+    ).first()
+
+    # [None] might seem unexpected, postgresql indeed returns [null]
+    assert annotated_result.json_array == [None]
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_no_related_objects_with_nested_output_field(faker: Faker):
+    """Test JSONArrayAgg aggregating when there are no related objects."""
+    author_name = faker.name()
+    Author.objects.create(name=author_name)
+
+    annotated_result = Author.objects.annotate(
+        json_array=JSONArrayAgg(
+            "posts__updated_at", nested_output_field=DateTimeField()
+        )
+    ).first()
+
+    # [None] might seem unexpected, postgresql indeed returns [null]
+    assert annotated_result.json_array == [None]
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_null_value(faker: Faker):
+    """Test JSONArrayAgg aggregating when related object has null value."""
+    author_name = faker.name()
+    post_title = faker.slug()
+    author = Author.objects.create(name=author_name)
+    Post.objects.create(title=post_title, author=author, content=None)
+
+    annotated_result = Author.objects.annotate(
+        json_array=JSONArrayAgg("posts__content")
+    ).first()
+
+    assert annotated_result.json_array == [None]
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_null_value_with_nested_output_field(faker: Faker):
+    """Test JSONArrayAgg aggregating when related object has null value."""
+    author_name = faker.name()
+    post_title = faker.slug()
+    author = Author.objects.create(name=author_name)
+    Post.objects.create(title=post_title, author=author, updated_at=None)
+
+    annotated_result = Author.objects.annotate(
+        json_array=JSONArrayAgg(
+            "posts__updated_at", nested_output_field=DateTimeField()
+        )
+    ).first()
+
+    assert annotated_result.json_array == [None]
+    assert annotated_result.name == author_name

tests/test_obj_agg.py

@@ -8,9 +8,12 @@
 
 import pytest
 from django.db.models import DateTimeField
+from django.db.models import JSONField
+from django.db.models import Q
 
 from json_agg import JSONObjectAgg
 from tests.models import Author
+from tests.models import Post
 from tests.post_factory import post_factory
 
 
@@ -99,3 +102,132 @@ def test_aggregate_json(faker: Faker):
 
     result_as_dict = {author.name: author.json_obj for author in queryset}
     assert result_as_dict == expected_value_per_author_name
+
+
+@pytest.mark.django_db
+def test_aggregate_json_with_nested_output_field(faker: Faker, db_vendor: str):
+    """Test JSONObjectAgg over a json value (Post.metadata)."""
+    if db_vendor != "sqlite":
+        pytest.skip("only sqlite is supported in this test.")
+    expected_value_per_author_name = post_factory(
+        faker,
+        value_name="metadata",
+        value_factory=partial(faker.pydict, allowed_types=(str, int)),
+    )
+    queryset = Author.objects.annotate(
+        json_obj=JSONObjectAgg(
+            "posts__title",
+            "posts__metadata",
+            nested_output_field=JSONField(),
+        )
+    ).all()
+
+    result_as_dict = {author.name: author.json_obj for author in queryset}
+    assert result_as_dict == expected_value_per_author_name
+
+
+def test_raise_value_error_invalid_nested_output_field():
+    """Ensure ValueError is raised if invalid type is used for nested_output_field."""
+    with pytest.raises(ValueError):
+        JSONObjectAgg("foo", "bar", nested_output_field=True)
+
+
+@pytest.mark.django_db
+def test_with_no_related_objects(faker: Faker):
+    """Test JSONObjectAgg aggregating when there are no related objects."""
+    author_name = faker.name()
+    Author.objects.create(name=author_name)
+
+    annotated_result = Author.objects.annotate(
+        json_obj=JSONObjectAgg("posts__title", "posts__content")
+    ).first()
+
+    assert annotated_result.json_obj == {}
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_no_related_objects_with_nested_output_field(faker: Faker):
+    """Test JSONObjectAgg aggregating when there are no related objects."""
+    author_name = faker.name()
+    Author.objects.create(name=author_name)
+
+    annotated_result = Author.objects.annotate(
+        json_obj=JSONObjectAgg(
+            "posts__title", "posts__updated_at", nested_output_field=DateTimeField()
+        )
+    ).first()
+
+    assert annotated_result.json_obj == {}
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_null_key(faker: Faker, db_vendor: str):
+    """Test JSONObjectAgg aggregating when related object has a null key."""
+    author_name = faker.name()
+    author = Author.objects.create(name=author_name)
+    Post.objects.create(title=None, content=faker.paragraph(), author=author)
+
+    annotated_result = Author.objects.annotate(
+        json_obj=JSONObjectAgg("posts__title", "posts__content")
+    ).first()
+
+    # null keys are not supported, so the object will be completely ignored in this case
+    assert annotated_result.json_obj == {}
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_null_value(faker: Faker):
+    """Test JSONObjectAgg aggregating when related object has null value."""
+    author_name = faker.name()
+    post_title = faker.slug()
+    author = Author.objects.create(name=author_name)
+    Post.objects.create(title=post_title, author=author, content=None)
+
+    annotated_result = Author.objects.annotate(
+        json_obj=JSONObjectAgg("posts__title", "posts__content")
+    ).first()
+
+    assert annotated_result.json_obj == {post_title: None}
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_null_value_with_nested_output_field(faker: Faker):
+    """Test JSONObjectAgg aggregating when related object has null value."""
+    author_name = faker.name()
+    post_title = faker.slug()
+    author = Author.objects.create(name=author_name)
+    Post.objects.create(title=post_title, author=author, updated_at=None)
+
+    annotated_result = Author.objects.annotate(
+        json_obj=JSONObjectAgg(
+            "posts__title", "posts__updated_at", nested_output_field=DateTimeField()
+        )
+    ).first()
+
+    assert annotated_result.json_obj == {post_title: None}
+    assert annotated_result.name == author_name
+
+
+@pytest.mark.django_db
+def test_with_filter(faker: Faker):
+    """Test JSONObjectAgg with a explicit internal filter."""
+    # this test is important due to the use of filter in JSONObjectAgg internals
+    author_name = faker.name()
+    post_title = faker.slug()
+    post_content = faker.paragraph()
+    author = Author.objects.create(name=author_name)
+    Post.objects.create(title=post_title, author=author, content=post_content)
+    # create the post that will be ignored
+    Post.objects.create(title=faker.slug(), author=author, content=faker.paragraph())
+
+    annotated_result = Author.objects.annotate(
+        json_obj=JSONObjectAgg(
+            "posts__title", "posts__content", filter=Q(posts__title=post_title)
+        )
+    ).first()
+
+    assert annotated_result.json_obj == {post_title: post_content}