From c9b8593975f1d88741ff3ab44704d57c8bb904d9 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 1 May 2024 17:58:47 +0400 Subject: [PATCH] Handle "sa" and "publicdomain" licenses (#4220) --- api/api/models/media.py | 19 +++++++---- api/api/serializers/media_serializers.py | 8 +++-- api/api/utils/watermark.py | 12 ++++--- .../src/openverse_attribution/attribution.py | 13 +++++--- .../src/openverse_attribution/license.py | 33 ++++++++++++++++--- .../tests/test_attribution.py | 24 +++++++------- .../tests/test_license.py | 5 +++ 7 files changed, 79 insertions(+), 35 deletions(-) diff --git a/api/api/models/media.py b/api/api/models/media.py index 8396957dd90..953c817a5ec 100644 --- a/api/api/models/media.py +++ b/api/api/models/media.py @@ -8,7 +8,6 @@ from django.utils.html import format_html from elasticsearch import Elasticsearch, NotFoundError -from openverse_attribution.attribution import get_attribution_text from openverse_attribution.license import License from api.constants.moderation import DecisionAction @@ -91,20 +90,26 @@ class AbstractMedia( meta_data = models.JSONField(blank=True, null=True) @property - def license_url(self) -> str: + def license_url(self) -> str | None: """A direct link to the license deed or legal terms.""" if self.meta_data and (url := self.meta_data.get("license_url")): return url - else: - return License(self.license.lower()).url(self.license_version) + try: + lic = License(self.license.lower()) + except ValueError: + return None + return lic.url(self.license_version) @property - def attribution(self) -> str: + def attribution(self) -> str | None: """Legally valid attribution for the media item in plain-text English.""" - return get_attribution_text( - self.license.lower(), + try: + lic = License(self.license) + except ValueError: + return None + return lic.attribution( self.title, self.creator, self.license_version, diff --git a/api/api/serializers/media_serializers.py b/api/api/serializers/media_serializers.py index 4ec3807ffb3..a4f7df4d009 100644 --- a/api/api/serializers/media_serializers.py +++ b/api/api/serializers/media_serializers.py @@ -741,9 +741,11 @@ def to_representation(self, *args, **kwargs): output["license"] = output["license"].lower() if output.get("license_url") is None: - output["license_url"] = License(output["license"]).url( - output["license_version"] - ) + try: + lic = License(output["license"]) + output["license_url"] = lic.url(output["license_version"]) + except ValueError: + pass # Ensure URLs have scheme url_fields = ["url", "creator_url", "foreign_landing_url"] diff --git a/api/api/utils/watermark.py b/api/api/utils/watermark.py index 3f82ff8f66d..a2249bf430f 100644 --- a/api/api/utils/watermark.py +++ b/api/api/utils/watermark.py @@ -9,7 +9,7 @@ from rest_framework.exceptions import APIException import requests -from openverse_attribution.attribution import get_attribution_text +from openverse_attribution.license import License from PIL import Image, ImageDraw, ImageFont, UnidentifiedImageError from sentry_sdk import capture_exception @@ -174,6 +174,11 @@ def _print_attribution_on_image(img: Image.Image, image_info): :return: return the framed and attributed image """ + try: + lic = License(image_info["license"]) + except ValueError: + return img + width, height = img.size smaller_dimension = _smaller_dimension(width, height) @@ -190,12 +195,11 @@ def _print_attribution_on_image(img: Image.Image, image_info): font = ImageFont.truetype(_get_font_path(), size=font_size) - text = get_attribution_text( - image_info["license"], + text = lic.attribution( image_info["title"], image_info["creator"], image_info["license_version"], - license_url=False, + False, ) text = _fit_in_width(text, font, new_width) attribution_height = _get_attribution_height(text, font) diff --git a/packages/python/openverse-attribution/src/openverse_attribution/attribution.py b/packages/python/openverse-attribution/src/openverse_attribution/attribution.py index 453801557ba..5d65fef6d21 100644 --- a/packages/python/openverse-attribution/src/openverse_attribution/attribution.py +++ b/packages/python/openverse-attribution/src/openverse_attribution/attribution.py @@ -1,10 +1,15 @@ +from __future__ import annotations + import re +from typing import TYPE_CHECKING + -from openverse_attribution.license import License +if TYPE_CHECKING: + from openverse_attribution.license import License def get_attribution_text( - license_slug: str, + lic: License, title: str | None = None, creator: str | None = None, license_version: str | None = None, @@ -22,16 +27,14 @@ def get_attribution_text( To remove the sentence for viewing the legal text, set the ``license_url`` parameter to ``False``. + :param lic: the ``License`` enum instance for the work :param title: the name of the work, if known :param creator: the name of the work's creator, if known - :param license_slug: :param license_version: the version of the license, if known :param license_url: the URL to the license, to override the default :return: the plain-text English language attribution """ - lic = License(license_slug) - title = f'"{title}"' if title else "This work" attribution_template = "{title} {creator} {marked-licensed} {license}. {view-legal}" diff --git a/packages/python/openverse-attribution/src/openverse_attribution/license.py b/packages/python/openverse-attribution/src/openverse_attribution/license.py index 272f911ce35..f83fc51b377 100644 --- a/packages/python/openverse-attribution/src/openverse_attribution/license.py +++ b/packages/python/openverse-attribution/src/openverse_attribution/license.py @@ -1,5 +1,7 @@ from enum import StrEnum +from openverse_attribution.attribution import get_attribution_text + class License(StrEnum): """ @@ -17,6 +19,7 @@ class License(StrEnum): BY_NC_ND = "by-nc-nd" # Deprecated CC licenses + SA = "sa" SAMPLING = "sampling+" NC_SAMPLING = "nc-sampling+" @@ -25,6 +28,7 @@ class License(StrEnum): # Public domain mark PDM = "pdm" + PUBLIC_DOMAIN = "publicdomain" def name(self, version: str | None = None) -> str: """ @@ -37,7 +41,7 @@ def name(self, version: str | None = None) -> str: :return: the full name of the license """ - if self is License.PDM: + if self in {License.PDM, License.PUBLIC_DOMAIN}: name = "Public Domain Mark" else: name = self.value.upper().replace("SAMPLING", "Sampling") @@ -62,7 +66,7 @@ def url(self, version: str | None = None) -> str: if self is License.CC0: fragment = "publicdomain/zero/1.0" - elif self is License.PDM: + elif self in {License.PUBLIC_DOMAIN, License.PDM}: fragment = "publicdomain/mark/1.0" elif self.is_deprecated: fragment = f"licenses/{self}/1.0" @@ -70,6 +74,25 @@ def url(self, version: str | None = None) -> str: fragment = f"licenses/{self}/{version or '4.0'}" return f"https://creativecommons.org/{fragment}/" + def attribution( + self, + title: str | None = None, + creator: str | None = None, + version: str | None = None, + url: str | bool | None = None, + ): + """ + Get the attribution text for a media item released under this license. + + :param title: the name of the work, if known + :param creator: the name of the work's creator, if known + :param version: the version number of the license + :param url: the URL to the legal text of this license + :return: the plain-text English language attribution + """ + + return get_attribution_text(self, title, creator, version, url) + @property def is_deprecated(self) -> bool: """ @@ -79,7 +102,7 @@ def is_deprecated(self) -> bool: :return: whether this license has been deprecated """ - return self in {License.SAMPLING, License.NC_SAMPLING} + return self in {License.SAMPLING, License.NC_SAMPLING, License.SA} @property def is_pd(self) -> bool: @@ -90,7 +113,7 @@ def is_pd(self) -> bool: :return: whether a work with this license is in the public domain """ - return self in {License.PDM, License.CC0} + return self in {License.PUBLIC_DOMAIN, License.PDM, License.CC0} @property def is_cc(self) -> bool: @@ -102,4 +125,4 @@ def is_cc(self) -> bool: """ # Works because other than PDM, we only have CC licenses. - return self is not License.PDM + return self not in {License.PUBLIC_DOMAIN, License.PDM} diff --git a/packages/python/openverse-attribution/tests/test_attribution.py b/packages/python/openverse-attribution/tests/test_attribution.py index 4d5a5b2608f..5123ffd3d51 100644 --- a/packages/python/openverse-attribution/tests/test_attribution.py +++ b/packages/python/openverse-attribution/tests/test_attribution.py @@ -1,5 +1,5 @@ import pytest -from openverse_attribution.attribution import get_attribution_text +from openverse_attribution.license import License BLANK = object() @@ -13,36 +13,36 @@ "args, attribution", [ ( - ("by", "Title", "Creator", "0.0", "https://license/url"), # All known + ("Title", "Creator", "0.0", "https://license/url"), # All known '"Title" by Creator is licensed under CC BY 0.0. ' "To view a copy of this license, visit https://license/url.", ), ( - ("by", BLANK, "Creator", "0.0", "https://license/url"), # Unknown title + (BLANK, "Creator", "0.0", "https://license/url"), # Unknown title "This work by Creator is licensed under CC BY 0.0. " "To view a copy of this license, visit https://license/url.", ), ( - ("by", "Title", BLANK, "0.0", "https://license/url"), # Unknown creator + ("Title", BLANK, "0.0", "https://license/url"), # Unknown creator '"Title" is licensed under CC BY 0.0. ' "To view a copy of this license, visit https://license/url.", ), ( - ("by", "Title", "Creator", BLANK, "https://license/url"), # Unknown version + ("Title", "Creator", BLANK, "https://license/url"), # Unknown version '"Title" by Creator is licensed under CC BY. ' "To view a copy of this license, visit https://license/url.", ), ( - ("by", "Title", "Creator", "0.0", BLANK), # Unknown license URL + ("Title", "Creator", "0.0", BLANK), # Unknown license URL '"Title" by Creator is licensed under CC BY 0.0. ' "To view a copy of this license, visit https://creativecommons.org/licenses/by/0.0/.", ), ( - ("by", "Title", "Creator", "0.0", False), # Removed license URL + ("Title", "Creator", "0.0", False), # Removed license URL '"Title" by Creator is licensed under CC BY 0.0.', ), ( - ("by", BLANK, BLANK, BLANK, BLANK), # Almost all unknown + (BLANK, BLANK, BLANK, BLANK), # Almost all unknown "This work is licensed under CC BY. " "To view a copy of this license, visit https://creativecommons.org/licenses/by/4.0/.", ), @@ -50,11 +50,12 @@ ) def test_attribution_text( blank_val: str | None, - args: tuple[str, str, str, str, str], + args: tuple[str, str, str, str], attribution: str, ): + lic = License("by") args = (blank_val if arg is BLANK else arg for arg in args) - assert get_attribution_text(*args) == attribution + assert lic.attribution(*args) == attribution @pytest.mark.parametrize( @@ -76,4 +77,5 @@ def test_attribution_text_differentiates_license_and_other_tools( slug: str, attribution: str, ): - assert get_attribution_text(slug) == attribution + lic = License(slug) + assert lic.attribution() == attribution diff --git a/packages/python/openverse-attribution/tests/test_license.py b/packages/python/openverse-attribution/tests/test_license.py index 768add74c8a..ebf25e26ca3 100644 --- a/packages/python/openverse-attribution/tests/test_license.py +++ b/packages/python/openverse-attribution/tests/test_license.py @@ -14,6 +14,8 @@ def test_raises_value_error_on_invalid_license(): ("cc0", "2.0", "CC0 1.0"), ("pdm", None, "Public Domain Mark 1.0"), ("pdm", "2.0", "Public Domain Mark 1.0"), + ("sa", None, "CC SA 1.0"), + ("sa", "2.0", "CC SA 1.0"), ("sampling+", None, "CC Sampling+ 1.0"), ("sampling+", "2.0", "CC Sampling+ 1.0"), ("by", None, "CC BY"), @@ -32,6 +34,8 @@ def test_can_get_name_for_license(slug: str, version: str, name: str): ("cc0", "2.0", "publicdomain/zero/1.0/"), ("pdm", None, "publicdomain/mark/1.0/"), ("pdm", "2.0", "publicdomain/mark/1.0/"), + ("sa", None, "licenses/sa/1.0/"), + ("sa", "2.0", "licenses/sa/1.0/"), ("sampling+", None, "licenses/sampling+/1.0/"), ("sampling+", "2.0", "licenses/sampling+/1.0/"), ("by", None, "licenses/by/4.0/"), @@ -46,6 +50,7 @@ def test_can_get_url_for_license(slug: str, version: str, path: str): @pytest.mark.parametrize( "slug, is_dep", [ + ("sa", True), ("sampling+", True), ("nc-sampling+", True), ("by", False),