From db51bf8479edd8d43af21fbb7b725aa92a37dfd1 Mon Sep 17 00:00:00 2001 From: Tim <6887938+tim-schultz@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:47:01 -0600 Subject: [PATCH] =?UTF-8?q?refactor:=20store=20entire=20score=20object=20a?= =?UTF-8?q?nd=20return=20same=20score=20response=20as=E2=80=A6=20(#714)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: store entire score object and return same score response as other endpoints * chore: remaining tests for updated historical endpoint * chore: test score serialization --- api/registry/exceptions.py | 7 +- api/registry/models.py | 20 +- .../test/test_score_updated_serialization.py | 46 ++++ api/v2/api/api_stamps.py | 92 ++++++-- api/v2/test/test_historical_score_endpoint.py | 197 ++++++++++++++---- 5 files changed, 297 insertions(+), 65 deletions(-) create mode 100644 api/registry/test/test_score_updated_serialization.py diff --git a/api/registry/exceptions.py b/api/registry/exceptions.py index dee016263..aaf5bbcbd 100644 --- a/api/registry/exceptions.py +++ b/api/registry/exceptions.py @@ -53,11 +53,16 @@ class InvalidLimitException(APIException): default_detail = "Invalid limit." -class CreatedAtIsRequired(APIException): +class CreatedAtIsRequiredException(APIException): status_code = status.HTTP_400_BAD_REQUEST default_detail = "You must provide created_at as a query param." +class CreatedAtMalFormedException(APIException): + status_code = status.HTTP_400_BAD_REQUEST + default_detail = "Created at must be in the format YYYY-MM-DD." + + class NoRequiredPermissionsException(APIException): status_code = status.HTTP_403_FORBIDDEN default_detail = "You are not allowed to access this endpoint." diff --git a/api/registry/models.py b/api/registry/models.py index 6f2ec5fcc..f43a178b9 100644 --- a/api/registry/models.py +++ b/api/registry/models.py @@ -1,5 +1,7 @@ +import json from enum import Enum +from django.core import serializers from django.db import models from django.db.models.signals import pre_save from django.dispatch import receiver @@ -87,19 +89,29 @@ def __str__(self): return f"Score #{self.id}, score={self.score}, last_score_timestamp={self.last_score_timestamp}, status={self.status}, error={self.error}, evidence={self.evidence}, passport_id={self.passport_id}" +def serialize_score(score: Score): + json_score = {} + try: + serialized_score = serializers.serialize("json", [score]) + json_score = json.loads(serialized_score)[0] + except: + json_score["error"] = "Error serializing score" + + return json_score + + @receiver(pre_save, sender=Score) def score_updated(sender, instance, **kwargs): if instance.status != Score.Status.DONE: return instance + json_score = serialize_score(instance) + Event.objects.create( action=Event.Action.SCORE_UPDATE, address=instance.passport.address, community=instance.passport.community, - data={ - "score": float(instance.score) if instance.score is not None else 0, - "evidence": instance.evidence, - }, + data=json_score, ) return instance diff --git a/api/registry/test/test_score_updated_serialization.py b/api/registry/test/test_score_updated_serialization.py new file mode 100644 index 000000000..8f6abdcfb --- /dev/null +++ b/api/registry/test/test_score_updated_serialization.py @@ -0,0 +1,46 @@ +from unittest.mock import patch + +import pytest + +from registry.models import serialize_score + + +@pytest.mark.django_db +class TestSerializeScore: + def test_successful_serialization(self, scorer_score): + """Test successful serialization of a score object""" + # Use the existing scorer_score fixture + score = scorer_score + + # Serialize the score + result = serialize_score(score) + + # Verify the structure and content + assert isinstance(result, dict) + assert "model" in result + assert "fields" in result + assert "error" not in result + + # Verify the model name + assert ( + result["model"] == "registry.score" + ) # adjust if your model is named differently + + # Verify key fields are present + assert "status" in result["fields"] + assert "passport" in result["fields"] + + @patch("django.core.serializers.serialize") + def test_serialization_error(self, mock_serialize, scorer_score): + """Test handling of serialization errors""" + # Use the existing scorer_score fixture + score = scorer_score + + # Make serialize throw an exception + mock_serialize.side_effect = Exception("Serialization failed") + + # Attempt to serialize + result = serialize_score(score) + + # Verify error handling + assert result == {"error": "Error serializing score"} diff --git a/api/v2/api/api_stamps.py b/api/v2/api/api_stamps.py index ff3de4a08..230f08ff2 100644 --- a/api/v2/api/api_stamps.py +++ b/api/v2/api/api_stamps.py @@ -1,10 +1,11 @@ -from datetime import datetime +from datetime import datetime, time from decimal import Decimal -from typing import List +from typing import Any, Dict, List from urllib.parse import urljoin from django.conf import settings from django.core.cache import cache +from ninja import Schema from ninja_extra.exceptions import APIException import api_logging as logging @@ -32,7 +33,8 @@ fetch_all_stamp_metadata, ) from registry.exceptions import ( - CreatedAtIsRequired, + CreatedAtIsRequiredException, + CreatedAtMalFormedException, InternalServerErrorException, InvalidAddressException, InvalidAPIKeyPermissions, @@ -104,11 +106,50 @@ async def a_submit_passport(request, scorer_id: int, address: str) -> V2ScoreRes raise InternalServerErrorException("Unexpected error while submitting passport") +def process_date_parameter(date_str: str) -> datetime: + """ + Convert a date string (YYYY-MM-DD) to a datetime object set to the end of that day. + + Args: + date_str: String in format 'YYYY-MM-DD' + + Returns: + datetime: Datetime object set to 23:59:59 of the given date + + Raises: + ValueError: If date string is not in correct format + """ + try: + # Parse the date string + date_obj = datetime.strptime(date_str, "%Y-%m-%d") + # Set time to end of day (23:59:59) + return datetime.combine(date_obj.date(), time(23, 59, 59)) + except Exception: + raise CreatedAtMalFormedException() + + +def extract_score_data(event_data: Dict[str, Any]) -> Dict[str, Any]: + """ + Extract score data from either the legacy or new data structure. + + Args: + event_data: Dictionary containing score event data + + Returns: + Dictionary with normalized score data + """ + # Handle legacy format (with 'fields' key) + if "fields" in event_data: + return event_data["fields"] + # Handle new format (direct score data) + return event_data + + @api_router.get( "/stamps/{int:scorer_id}/score/{str:address}/history", auth=ApiKey(), response={ - 200: DetailedScoreResponse | NoScoreResponse, + 200: V2ScoreResponse | NoScoreResponse, 401: ErrorMessageResponse, 400: ErrorMessageResponse, 404: ErrorMessageResponse, @@ -122,7 +163,6 @@ async def a_submit_passport(request, scorer_id: int, address: str) -> V2ScoreRes \n To access this endpoint, you must submit your use case and be approved by the Passport team. To do so, please fill out the following form, making sure to provide a detailed description of your use case. The Passport team typically reviews and responds to form responses within 48 hours. https://forms.gle/4GyicBfhtHW29eEu8 """, - include_in_schema=False, tags=["Stamp Analysis"], ) @track_apikey_usage(track_response=False) @@ -130,27 +170,23 @@ def get_score_history( request, scorer_id: int, address: str, - created_at: str = "", + created_at: str, ): if not request.api_key.historical_endpoint: raise InvalidAPIKeyPermissions() - if not created_at: - raise CreatedAtIsRequired() + raise CreatedAtIsRequiredException() check_rate_limit(request) - community = api_get_object_or_404(Community, id=scorer_id, account=request.auth) try: + end_of_day = process_date_parameter(created_at) base_query = with_read_db(Event).filter( community__id=community.id, action=Event.Action.SCORE_UPDATE ) - score_event = ( - base_query.filter( - address=address, created_at__lte=datetime.fromisoformat(created_at) - ) + base_query.filter(address=address, created_at__lte=end_of_day) .order_by("-created_at") .first() ) @@ -160,16 +196,28 @@ def get_score_history( address=address, status=f"No Score Found for {address} at {created_at}" ) - # TODO: geri this is not correct, we need to review the return structure and value here - return DetailedScoreResponse( + # Extract and normalize score data from either format + score_data = extract_score_data(score_event.data) + + # Get evidence data, defaulting to empty dict if not present + evidence = score_data.get("evidence", {}) + threshold = evidence.get("threshold", "0") + + # Handle score extraction for both formats + if "evidence" in score_data and "rawScore" in score_data["evidence"]: + score = score_data["evidence"]["rawScore"] + else: + score = score_data.get("score", "0") + + return V2ScoreResponse( address=address, - score=score_event.data["score"], - status=Score.Status.DONE, - last_score_timestamp=score_event.data["created_at"], - expiration_date=score_event.data["expiration_date"], - evidence=score_event.data["evidence"], - error=None, - stamp_scores=None, + score=score, + passing_score=(Decimal(score) >= Decimal(threshold) if score else False), + threshold=threshold, + last_score_timestamp=score_data.get("last_score_timestamp"), + expiration_timestamp=score_data.get("expiration_date"), + error=score_data.get("error"), + stamp_scores=score_data.get("stamp_scores"), ) except Exception as e: diff --git a/api/v2/test/test_historical_score_endpoint.py b/api/v2/test/test_historical_score_endpoint.py index 4590efc7d..be4639d38 100644 --- a/api/v2/test/test_historical_score_endpoint.py +++ b/api/v2/test/test_historical_score_endpoint.py @@ -1,4 +1,5 @@ from datetime import datetime, timedelta +from decimal import Decimal from unittest.mock import MagicMock, patch import pytest @@ -23,9 +24,10 @@ def test_get_historical_score_invalid_api_key_permissions( ): client = Client() response = client.get( - f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history", + f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=2024-10-26", HTTP_AUTHORIZATION="Token " + scorer_api_key_no_permissions, ) + assert response.status_code == 403 def test_get_historical_score_required_created_at( @@ -40,43 +42,162 @@ def test_get_historical_score_required_created_at( HTTP_AUTHORIZATION="Token " + scorer_api_key, ) response_data = response.json() - assert response.status_code == 400 + assert response.status_code == 422 + assert response_data["detail"] == [ + {"type": "missing", "loc": ["query", "created_at"], "msg": "Field required"} + ] + + @freeze_time("2023-01-01") + def test_get_historical_score_legacy_format( + self, + scorer_account, + scorer_api_key, + scorer_community_with_binary_scorer, + ): + base_date = datetime(2023, 1, 1) + # Create score event with legacy format + Event.objects.create( + action=Event.Action.SCORE_UPDATE, + address=scorer_account.address, + community=scorer_community_with_binary_scorer, + data={ + "score": 1.0, + "evidence": { + "type": "ThresholdScoreCheck", + "success": True, + "rawScore": "309.5190000000000054014570595", + "threshold": "100.00000", + }, + }, + ) + + client = Client() + response = client.get( + f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=2023-01-01", + HTTP_AUTHORIZATION="Token " + scorer_api_key, + ) + response_data = response.json() + + assert response.status_code == 200 + assert response_data["score"] == "309.5190000000000054014570595" + assert response_data["threshold"] == "100.00000" + assert response_data["passing_score"] is True + assert response_data["last_score_timestamp"] == None + assert response_data["expiration_timestamp"] == None + + @freeze_time("2023-01-01") + def test_get_historical_score_new_format( + self, + scorer_account, + scorer_api_key, + scorer_community_with_binary_scorer, + ): + base_date = datetime(2023, 1, 1) + # Create score event with new format + Event.objects.create( + action=Event.Action.SCORE_UPDATE, + address=scorer_account.address, + community=scorer_community_with_binary_scorer, + data={ + "pk": 15, + "model": "registry.score", + "fields": { + "error": None, + "score": "0", + "status": "DONE", + "evidence": { + "type": "ThresholdScoreCheck", + "success": False, + "rawScore": "5.459000000000000019095836024", + "threshold": "100.00000", + }, + "passport": 15, + "stamp_scores": {"github": 10, "twitter": 15}, + "expiration_date": "2024-11-21T22:09:10.687Z", + "last_score_timestamp": "2024-10-25T19:16:14.023Z", + }, + }, + ) + + client = Client() + response = client.get( + f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=2023-01-01", + HTTP_AUTHORIZATION="Token " + scorer_api_key, + ) + response_data = response.json() + + assert response.status_code == 200 + assert response_data["score"] == "5.459000000000000019095836024" + assert response_data["threshold"] == "100.00000" + assert response_data["passing_score"] is False + assert response_data["last_score_timestamp"] == "2024-10-25T19:16:14.023Z" + assert response_data["expiration_timestamp"] == "2024-11-21T22:09:10.687Z" + assert response_data["stamp_scores"] == {"github": "10", "twitter": "15"} + + @freeze_time("2023-01-01") + def test_get_historical_score_missing_fields( + self, + scorer_account, + scorer_api_key, + scorer_community_with_binary_scorer, + ): + # Create score event with minimal data + Event.objects.create( + action=Event.Action.SCORE_UPDATE, + address=scorer_account.address, + community=scorer_community_with_binary_scorer, + data={ + "evidence": { + "rawScore": "15", + }, + }, + ) + + client = Client() + response = client.get( + f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=2023-01-01", + HTTP_AUTHORIZATION="Token " + scorer_api_key, + ) + response_data = response.json() + + assert response.status_code == 200 + assert response_data["score"] == "15" + assert response_data["threshold"] == "0" + assert response_data["passing_score"] is True + assert response_data["last_score_timestamp"] is None + assert response_data["expiration_timestamp"] is None + assert response_data["stamp_scores"] is None + + def test_get_historical_score_no_score_found( + self, + scorer_account, + scorer_api_key, + scorer_community_with_binary_scorer, + ): + client = Client() + response = client.get( + f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=2023-01-01", + HTTP_AUTHORIZATION="Token " + scorer_api_key, + ) + response_data = response.json() + + assert response.status_code == 200 + assert response_data["address"] == scorer_account.address assert ( - response_data["detail"] == "You must provide created_at as a query param." + response_data["status"] + == f"No Score Found for {scorer_account.address} at 2023-01-01" ) - # @freeze_time("2023-01-01") - # def test_get_historical_score_address_at_timestamp( - # self, - # scorer_account, - # scorer_api_key, - # scorer_community_with_binary_scorer, - # ): - # base_date = datetime(2023, 1, 1) - # # Create multiple score events - # for i in range(3): - # with freeze_time(base_date + timedelta(days=i)): - # Event.objects.create( - # action=Event.Action.SCORE_UPDATE, - # address=scorer_account.address, - # community=scorer_community_with_binary_scorer, - # data={ - # "score": i, - # "evidence": { - # "rawScore": 20 + i, - # "type": "binary", - # "success": True, - # "threshold": 5, - # }, - # }, - # ) - - # client = Client() - # response = client.get( - # f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=2023-01-01T00:00:00", - # HTTP_AUTHORIZATION="Token " + scorer_api_key, - # ) - # response_data = response.json() - - # assert response.status_code == 200 - # assert response_data["score"] == "0" + def test_get_historical_score_invalid_date( + self, + scorer_account, + scorer_api_key, + scorer_community_with_binary_scorer, + ): + client = Client() + response = client.get( + f"{self.base_url}/{scorer_community_with_binary_scorer.id}/score/{scorer_account.address}/history?created_at=invalid-date", + HTTP_AUTHORIZATION="Token " + scorer_api_key, + ) + + assert response.status_code == 400