Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Reduce DB queries needed in search results #1040

Merged
merged 23 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
efdeb29
Use FK and O2O instead of 'identifier' in reporting models
dhruvkb Dec 12, 2022
834e7dc
Add support for selecting related to reduce N+1 queries
dhruvkb Dec 12, 2022
f30923e
Use `ForeignObject` to link audio with audio set
dhruvkb Dec 12, 2022
a0fbc5f
Fix the order of save operations
dhruvkb Dec 12, 2022
235e305
Fix incorrect field name
dhruvkb Dec 12, 2022
610ec6b
Use relational fields for serializing report objects
dhruvkb Dec 12, 2022
0105e8c
Fix failing tests
dhruvkb Dec 12, 2022
6473b7c
Avoid mapping of hits to DB for dead link filtering tests
dhruvkb Dec 12, 2022
c3681fe
Explain reason for patching `ImageSerializer`
dhruvkb Dec 13, 2022
516925b
Remove leftover debugging statement
dhruvkb Dec 13, 2022
640211e
Restore `fields_matched` field
dhruvkb Dec 13, 2022
121f82b
Handle hits without `fields_matched`
dhruvkb Dec 13, 2022
131fd22
Merge branch 'main' of https://github.com/WordPress/openverse-api int…
dhruvkb Dec 13, 2022
830d73e
Test query count for single result view
dhruvkb Dec 13, 2022
a6e3759
Reduce frivolous changes
dhruvkb Dec 13, 2022
77f1801
Use `api_client` instead of factory
dhruvkb Dec 14, 2022
b615a29
Add additional test for the search results endpoint
dhruvkb Dec 14, 2022
48c463f
Return 20 results instead of 1
dhruvkb Dec 16, 2022
523338d
Reduce the migration complexity
dhruvkb Dec 20, 2022
038e1b9
Avoid changes to the DB schema
dhruvkb Dec 20, 2022
870e5eb
Lock down the `db_column` name for a future name-change
dhruvkb Dec 20, 2022
1da549f
Use `RenameField` for sensible field names
dhruvkb Dec 20, 2022
03e30f8
Merge branch 'main' into select_related
dhruvkb Dec 20, 2022
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
50 changes: 34 additions & 16 deletions api/catalog/api/admin/__init__.py
Original file line number Diff line number Diff line change
@@ -1,40 +1,61 @@
from django.contrib import admin

from catalog.api.admin.site import openverse_admin
from catalog.api.models import PENDING, AudioReport, ContentProvider, ImageReport
from catalog.api.models import (
PENDING,
Audio,
AudioReport,
ContentProvider,
Image,
ImageReport,
)
from catalog.api.models.media import AbstractDeletedMedia, AbstractMatureMedia


admin.site = openverse_admin
admin.sites.site = openverse_admin


@admin.register(Image)
class ImageAdmin(admin.ModelAdmin):
search_fields = ("identifier",)


@admin.register(Audio)
class AudioAdmin(admin.ModelAdmin):
search_fields = ("identifier",)


class MediaReportAdmin(admin.ModelAdmin):
list_display = ("reason", "status", "description", "created_at")
media_specific_list_display = ()
list_filter = ("status", "reason")
list_display_links = ("status",)
search_fields = ("description", "identifier")
search_fields = ("description", "media_obj__identifier")
autocomplete_fields = ("media_obj",)
actions = None

def get_list_display(self, request):
return self.list_display + self.media_specific_list_display

def get_exclude(self, request, obj=None):
# ``identifier`` cannot be edited on an existing report.
if request.path.endswith("/change/"):
return ["media_obj"]

def get_readonly_fields(self, request, obj=None):
if obj is None:
return []
always_readonly = [
readonly_fields = [
"reason",
"description",
"identifier",
"media_obj_id",
"created_at",
]
if obj.status == PENDING:
return always_readonly
else:
status_readonly = ["status"]
status_readonly.extend(always_readonly)
return status_readonly
# ``status`` cannot be changed on a finalised report.
if obj.status != PENDING:
readonly_fields.append("status")
return readonly_fields


@admin.register(ImageReport)
Expand All @@ -48,12 +69,9 @@ class AudioReportAdmin(MediaReportAdmin):


class MediaSubreportAdmin(admin.ModelAdmin):
search_fields = [
"identifier",
]
readonly_fields = [
"identifier",
]
exclude = ("media_obj",)
search_fields = ("media_obj__identifier",)
readonly_fields = ("media_obj_id",)

def has_add_permission(self, *args, **kwargs):
"""Create ``_Report`` instances instead."""
Expand Down
12 changes: 11 additions & 1 deletion api/catalog/api/admin/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,17 @@ def get_app_list(self, *args, **kwargs):
if media_type in model["object_name"].lower():
models.append(model)
api_app["models"].remove(model)
models.sort(key=lambda x: "0" if "report" in x["name"] else x["name"])

def key(entry):
name = entry["name"]
if name in ["Audios", "Images"]:
return "0"
elif "report" in name:
return "1"
else:
return name

models.sort(key=key)

media_app = {
"name": media_type_name,
Expand Down
8 changes: 1 addition & 7 deletions api/catalog/api/examples/audio_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,7 @@
"page_size": 20,
"page": 1,
"results": [
base_audio
| {
"fields_matched": ["title"],
"audio_set": None,
"attribution": None,
"filesize": None,
dhruvkb marked this conversation as resolved.
Show resolved Hide resolved
},
base_audio | {"fields_matched": ["title"]},
],
},
}
Expand Down
26 changes: 10 additions & 16 deletions api/catalog/api/examples/image_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
"provider": "stocksnap",
"source": "stocksnap",
"category": "photograph",
"filesize": None,
"filetype": None,
"filesize": 896128,
"filetype": "jpg",
"tags": [
{"accuracy": None, "name": "tree"},
{"accuracy": None, "name": "bark"},
Expand All @@ -43,11 +43,15 @@
{"accuracy": None, "name": "closeup"},
{"accuracy": None, "name": "root"},
],
"attribution": None,
"attribution": (
'"Tree Bark Photo" by Tim Sullivan is marked with '
"CC0 1.0. To view the terms, visit "
"https://creativecommons.org/publicdomain/zero/1.0/."
),
"fields_matched": [],
"mature": False,
"height": None,
"width": None,
"height": 4016,
"width": 6016,
"thumbnail": f"{origin}/v1/images/{identifier}/thumb/",
"detail_url": f"{origin}/v1/images/{identifier}/",
"related_url": f"{origin}/v1/images/{identifier}/related/",
Expand All @@ -67,17 +71,7 @@
"page_count": 0,
"page_size": 20,
"page": 1,
"results": [
base_image
| {
"fields_matched": ["title"],
"height": None,
"width": None,
"attribution": None,
"filesize": None,
"filetype": None,
}
],
"results": [base_image | {"fields_matched": ["title"]}],
},
}

Expand Down
79 changes: 79 additions & 0 deletions api/catalog/api/migrations/0052_relational_fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Generated by Django 4.1.3 on 2022-12-20 06:17

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('api', '0051_delete_sourcelogo'),
]

operations = [
migrations.AddField(
model_name='audio',
name='audioset',
field=models.ForeignObject(from_fields=('audio_set_foreign_identifier', 'provider'), null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='api.audioset', to_fields=('foreign_identifier', 'provider')),
),
migrations.AlterField(
model_name='audioreport',
name='identifier',
field=models.ForeignKey(db_column="identifier", db_constraint=False, help_text='The reference to the audio being reported.', on_delete=django.db.models.deletion.DO_NOTHING, related_name='audio_report', to='api.audio', to_field='identifier'),
),
migrations.AlterField(
model_name='deletedaudio',
name='identifier',
field=models.OneToOneField(db_column="identifier", db_constraint=False, help_text='The reference to the deleted audio.', on_delete=django.db.models.deletion.DO_NOTHING, primary_key=True, related_name='deleted_audio', serialize=False, to='api.audio', to_field='identifier'),
),
migrations.AlterField(
model_name='deletedimage',
name='identifier',
field=models.OneToOneField(db_column="identifier", db_constraint=False, help_text='The reference to the deleted image.', on_delete=django.db.models.deletion.DO_NOTHING, primary_key=True, related_name='deleted_image', serialize=False, to='api.image', to_field='identifier'),
),
migrations.AlterField(
model_name='imagereport',
name='identifier',
field=models.ForeignKey(db_column="identifier", db_constraint=False, help_text='The reference to the image being reported.', on_delete=django.db.models.deletion.DO_NOTHING, related_name='image_report', to='api.image', to_field='identifier'),
),
migrations.AlterField(
model_name='matureaudio',
name='identifier',
field=models.OneToOneField(db_column="identifier", db_constraint=False, help_text='The reference to the mature audio.', on_delete=django.db.models.deletion.DO_NOTHING, primary_key=True, related_name='mature_audio', serialize=False, to='api.audio', to_field='identifier'),
),
migrations.AlterField(
model_name='matureimage',
name='identifier',
field=models.OneToOneField(db_column="identifier", db_constraint=False, help_text='The reference to the mature image.', on_delete=django.db.models.deletion.DO_NOTHING, primary_key=True, related_name='mature_image', serialize=False, to='api.image', to_field='identifier'),
),
migrations.RenameField(
model_name="audioreport",
old_name="identifier",
new_name="media_obj",
),
migrations.RenameField(
model_name="deletedaudio",
old_name="identifier",
new_name="media_obj",
),
migrations.RenameField(
model_name="deletedimage",
old_name="identifier",
new_name="media_obj",
),
migrations.RenameField(
model_name="imagereport",
old_name="identifier",
new_name="media_obj",
),
migrations.RenameField(
model_name="matureaudio",
old_name="identifier",
new_name="media_obj",
),
migrations.RenameField(
model_name="matureimage",
old_name="identifier",
new_name="media_obj",
),
]
50 changes: 42 additions & 8 deletions api/catalog/api/models/audio.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ class Audio(AudioFileMixin, AbstractMedia):
category: eg. music, sound_effect, podcast, news & audiobook
"""

audioset = models.ForeignObject(
to="AudioSet",
on_delete=models.DO_NOTHING,
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
from_fields=["audio_set_foreign_identifier", "provider"],
to_fields=["foreign_identifier", "provider"],
null=True,
)

# Replaces the foreign key to AudioSet
audio_set_foreign_identifier = models.CharField(
max_length=1000,
Expand Down Expand Up @@ -178,7 +186,7 @@ class Audio(AudioFileMixin, AbstractMedia):

@property
def mature(self) -> bool:
return MatureAudio.objects.filter(identifier=self.identifier).exists()
return hasattr(self, "mature_audio")

@property
def alternative_files(self):
Expand All @@ -192,13 +200,7 @@ def duration_in_s(self):

@property
def audio_set(self):
try:
return AudioSet.objects.get(
provider=self.provider,
foreign_identifier=self.audio_set_foreign_identifier,
)
except AudioSet.DoesNotExist:
return None
return getattr(self, "audioset")

def get_waveform(self) -> list[float]:
"""
Expand Down Expand Up @@ -236,6 +238,17 @@ class DeletedAudio(AbstractDeletedMedia):
media_class = Audio
es_index = settings.MEDIA_INDEX_MAPPING[AUDIO_TYPE]

media_obj = models.OneToOneField(
to="Audio",
to_field="identifier",
on_delete=models.DO_NOTHING,
primary_key=True,
db_constraint=False,
db_column="identifier",
related_name="deleted_audio",
help_text="The reference to the deleted audio.",
)

class Meta:
verbose_name_plural = "Deleted audio"

Expand All @@ -249,6 +262,17 @@ class MatureAudio(AbstractMatureMedia):
media_class = Audio
es_index = settings.MEDIA_INDEX_MAPPING[AUDIO_TYPE]

media_obj = models.OneToOneField(
to="Audio",
to_field="identifier",
on_delete=models.DO_NOTHING,
primary_key=True,
db_constraint=False,
db_column="identifier",
related_name="mature_audio",
help_text="The reference to the mature audio.",
)

class Meta:
verbose_name_plural = "Mature audio"

Expand All @@ -258,6 +282,16 @@ class AudioReport(AbstractMediaReport):
mature_class = MatureAudio
deleted_class = DeletedAudio

media_obj = models.ForeignKey(
to="Audio",
to_field="identifier",
on_delete=models.DO_NOTHING,
db_constraint=False,
db_column="identifier",
related_name="audio_report",
help_text="The reference to the audio being reported.",
)

class Meta:
db_table = "nsfw_reports_audio"

Expand Down
Loading