From e93e5ca12c1a9d1b8b61897fd3facbc67c2ddb52 Mon Sep 17 00:00:00 2001 From: Christopher Horn Date: Mon, 9 Nov 2020 11:48:21 -0500 Subject: [PATCH 1/3] Refactor /api/relations and add unit tests --- gramps_webapi/api/__init__.py | 7 +- gramps_webapi/api/resources/relation.py | 77 ++++++++++++---- gramps_webapi/data/apispec.yaml | 62 +++++++++++-- tests/test_endpoints/test_relations.py | 114 ++++++++++++++++++++++++ 4 files changed, 233 insertions(+), 27 deletions(-) create mode 100644 tests/test_endpoints/test_relations.py diff --git a/gramps_webapi/api/__init__.py b/gramps_webapi/api/__init__.py index e7ce5517..283a9399 100644 --- a/gramps_webapi/api/__init__.py +++ b/gramps_webapi/api/__init__.py @@ -21,7 +21,7 @@ from .resources.note import NoteResource, NotesResource from .resources.person import PeopleResource, PersonResource from .resources.place import PlaceResource, PlacesResource -from .resources.relation import RelationResource +from .resources.relation import RelationResource, RelationsResource from .resources.repository import RepositoriesResource, RepositoryResource from .resources.source import SourceResource, SourcesResource from .resources.tag import TagResource, TagsResource @@ -105,6 +105,11 @@ def register_endpt(resource: Type[Resource], url: str, name: str): register_endpt( RelationResource, "/relations//", + "relation", +) +register_endpt( + RelationsResource, + "/relations///all", "relations", ) # Metadata diff --git a/gramps_webapi/api/resources/relation.py b/gramps_webapi/api/resources/relation.py index cba67136..4397a028 100644 --- a/gramps_webapi/api/resources/relation.py +++ b/gramps_webapi/api/resources/relation.py @@ -3,6 +3,7 @@ from typing import Dict from flask import Response, abort +from gramps.gen.const import GRAMPS_LOCALE from gramps.gen.db.base import DbReadBase from gramps.gen.relationship import RelationshipCalculator from webargs import fields @@ -24,39 +25,32 @@ def db_handle(self) -> DbReadBase: return get_dbstate().db @use_args( - {"depth": fields.Integer(), "all": fields.Boolean()}, + {"depth": fields.Integer(), "locale": fields.Boolean(missing=False)}, location="query", ) def get(self, args: Dict, handle1: Handle, handle2: Handle) -> Response: - """Get the relationship between two people.""" + """Get the most direct relationship between two people.""" db_handle = self.db_handle person1 = get_person_by_handle(db_handle, handle1) - if person1 is None: + if person1 == {}: abort(404) person2 = get_person_by_handle(db_handle, handle2) - if person2 is None: + if person2 == {}: abort(404) calc = RelationshipCalculator() if "depth" in args: calc.set_depth(args["depth"]) - if "all" in args and args["all"]: - data = calc.get_all_relationships(db_handle, person1, person2) - index = 0 - result = [] - while index < len(data[0]): - result.append( - { - "relationship_string": data[0][index], - "common_ancestors": data[1][index], - } - ) - index = index + 1 - return self.response(200, result) - - data = calc.get_one_relationship(db_handle, person1, person2, extra_info=True) + if args["locale"]: + data = calc.get_one_relationship( + db_handle, person1, person2, extra_info=True, olocale=GRAMPS_LOCALE + ) + else: + data = calc.get_one_relationship( + db_handle, person1, person2, extra_info=True + ) return self.response( 200, { @@ -65,3 +59,48 @@ def get(self, args: Dict, handle1: Handle, handle2: Handle) -> Response: "distance_common_other": data[2], }, ) + + +class RelationsResource(ProtectedResource, GrampsJSONEncoder): + """Relations resource.""" + + @property + def db_handle(self) -> DbReadBase: + """Get the database instance.""" + return get_dbstate().db + + @use_args( + { + "depth": fields.Integer(), + }, + location="query", + ) + def get(self, args: Dict, handle1: Handle, handle2: Handle) -> Response: + """Get all possible relationships between two people.""" + db_handle = self.db_handle + person1 = get_person_by_handle(db_handle, handle1) + if person1 == {}: + abort(404) + + person2 = get_person_by_handle(db_handle, handle2) + if person2 == {}: + abort(404) + + calc = RelationshipCalculator() + if "depth" in args: + calc.set_depth(args["depth"]) + + data = calc.get_all_relationships(db_handle, person1, person2) + result = [] + index = 0 + while index < len(data[0]): + result.append( + { + "relationship_string": data[0][index], + "common_ancestors": data[1][index], + } + ) + index = index + 1 + if result == []: + result = [{}] + return self.response(200, result) diff --git a/gramps_webapi/data/apispec.yaml b/gramps_webapi/data/apispec.yaml index 6f79a2d6..74709221 100644 --- a/gramps_webapi/data/apispec.yaml +++ b/gramps_webapi/data/apispec.yaml @@ -2201,7 +2201,7 @@ paths: get: tags: - relations - summary: "Get relation between two people if one exists." + summary: "Get description of most direct relationship between two people if one exists." operationId: getRelation security: - Bearer: [] @@ -2221,11 +2221,11 @@ paths: required: false type: integer description: "Depth for the search, default is 15 generations." - - name: all + - name: locale in: query required: false type: boolean - description: "Indicates to return all the ways the people are related as opposed to the most direct way." + description: "Indicates to return relationship string in current locale." responses: 200: description: "OK: Successful operation." @@ -2234,6 +2234,38 @@ paths: 401: description: "Unauthorized: Missing authorization header." + /relations/{handle1}/{handle2}/all: + get: + tags: + - relations + summary: "Get descriptions for all possible relationships between two people if any exist." + operationId: getRelations + security: + - Bearer: [] + parameters: + - name: handle1 + in: path + required: true + type: string + description: "The handle of the first person." + - name: handle2 + in: path + required: true + type: string + description: "The handle of the second person." + - name: depth + in: query + required: false + type: integer + description: "Depth for the search, default is 15 generations." + responses: + 200: + description: "OK: Successful operation." + schema: + $ref: "#/definitions/Relationships" + 401: + description: "Unauthorized: Missing authorization header." + ############################################################################## # Endpoint - Metadata ############################################################################## @@ -4270,17 +4302,33 @@ definitions: relationship_string: description: "Descriptive string describing the relationship." type: string + example: "second great stepgrandaunt" distance_common_origin: description: "Number of generations to common ancestor, -1 if no common ancestor." type: integer + example: 5 distance_common_other: description: "Number of generations to other person in common, -1 if there is none." type: integer - common_ancestors: - description: "List of handles of common ancestors." - type: array - items: + example: 1 + + Relationships: + type: array + items: + type: object + properties: + relationship_string: + description: "Descriptive string describing the relationship." type: string + example: "second great stepgrandaunt" + common_ancestors: + description: "List of handles of common ancestors." + type: array + items: + type: string + example: + - 35WJQC1B7T7NPV8OLV + - 46WJQCIOLQ0KOX2XCC ############################################################################## # Model - TreeSummary diff --git a/tests/test_endpoints/test_relations.py b/tests/test_endpoints/test_relations.py new file mode 100644 index 00000000..6d41e7da --- /dev/null +++ b/tests/test_endpoints/test_relations.py @@ -0,0 +1,114 @@ +"""Tests for the /api/relations endpoints using example_gramps.""" + +import unittest + +from . import get_test_client + + +class TestRelation(unittest.TestCase): + """Test cases for the /api/relations/{handle1}/{handle2} endpoint.""" + + @classmethod + def setUpClass(cls): + """Test class setup.""" + cls.client = get_test_client() + + def test_relations_endpoint_404(self): + """Test response for missing or bad handles.""" + # check various handle issues + result = self.client.get("/api/relations/9BXKQC1PVLPYFMD6IX") + self.assertEqual(result.status_code, 404) + result = self.client.get("/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR1") + self.assertEqual(result.status_code, 404) + result = self.client.get("/api/relations/9BXKQC1PVLPYFMD6I/ORFKQC4KLWEGTGR19L") + self.assertEqual(result.status_code, 404) + + def test_relations_endpoint(self): + """Test response for valid request.""" + # check expected response which also confirms response schema + result = self.client.get("/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L") + self.assertEqual( + result.json, + { + "distance_common_origin": 5, + "distance_common_other": 1, + "relationship_string": "second great stepgrandaunt", + }, + ) + + def test_relations_endpoint_parms(self): + """Test responses for query parms.""" + # check bad or invalid query parm + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?junk=1" + ) + self.assertEqual(result.status_code, 422) + # check depth parm working as expected + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?depth=5" + ) + self.assertEqual(result.json["relationship_string"], "") + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?depth=6" + ) + self.assertEqual( + result.json["relationship_string"], "second great stepgrandaunt" + ) + + +class TestRelations(unittest.TestCase): + """Test cases for the /api/relations/{handle1}/{handle2}/all endpoint.""" + + @classmethod + def setUpClass(cls): + """Test class setup.""" + cls.client = get_test_client() + + def test_relations_all_endpoint_404(self): + """Test response for missing or bad handles.""" + # check various handle issues + result = self.client.get("/api/relations/9BXKQC1PVLPYFMD6IX/all") + self.assertEqual(result.status_code, 404) + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR1/all" + ) + self.assertEqual(result.status_code, 404) + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6I/ORFKQC4KLWEGTGR19L/all" + ) + self.assertEqual(result.status_code, 404) + + def test_relations_all_endpoint(self): + """Test response for valid request.""" + # check expected response which also confirms response schema + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all" + ) + self.assertEqual( + result.json, + [ + { + "common_ancestors": ["35WJQC1B7T7NPV8OLV", "46WJQCIOLQ0KOX2XCC"], + "relationship_string": "second great stepgrandaunt", + } + ], + ) + + def test_relations_all_endpoint_parms(self): + """Test responses for query parms.""" + # check bad or invalid query parm + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?junk=1" + ) + self.assertEqual(result.status_code, 422) + # check depth parm working as expected + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?depth=5" + ) + self.assertEqual(result.json, [{}]) + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?depth=6" + ) + self.assertEqual( + result.json[0]["relationship_string"], "second great stepgrandaunt" + ) From eb83e4027464591c4cf7982d9e7e2b0aaa607814 Mon Sep 17 00:00:00 2001 From: Christopher Horn Date: Sat, 14 Nov 2020 10:56:03 -0500 Subject: [PATCH 2/3] Add simple locale parm test --- tests/test_endpoints/test_relations.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_endpoints/test_relations.py b/tests/test_endpoints/test_relations.py index 6d41e7da..9c02b642 100644 --- a/tests/test_endpoints/test_relations.py +++ b/tests/test_endpoints/test_relations.py @@ -44,6 +44,10 @@ def test_relations_endpoint_parms(self): ) self.assertEqual(result.status_code, 422) # check depth parm working as expected + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?depth" + ) + self.assertEqual(result.status_code, 422) result = self.client.get( "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?depth=5" ) @@ -54,6 +58,15 @@ def test_relations_endpoint_parms(self): self.assertEqual( result.json["relationship_string"], "second great stepgrandaunt" ) + # check locale parm working + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?locale" + ) + self.assertEqual(result.status_code, 422) + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L?locale=1" + ) + self.assertEqual(result.status_code, 200) class TestRelations(unittest.TestCase): @@ -102,6 +115,10 @@ def test_relations_all_endpoint_parms(self): ) self.assertEqual(result.status_code, 422) # check depth parm working as expected + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?depth" + ) + self.assertEqual(result.status_code, 422) result = self.client.get( "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?depth=5" ) @@ -112,3 +129,12 @@ def test_relations_all_endpoint_parms(self): self.assertEqual( result.json[0]["relationship_string"], "second great stepgrandaunt" ) + # check locale parm working + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?locale" + ) + self.assertEqual(result.status_code, 422) + result = self.client.get( + "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?locale=1" + ) + self.assertEqual(result.status_code, 200) From 836f311eef50af136357fd04df47fb11f52e2da8 Mon Sep 17 00:00:00 2001 From: Christopher Horn Date: Sat, 14 Nov 2020 11:15:36 -0500 Subject: [PATCH 3/3] Remove locale from all test, forgot not supported --- tests/test_endpoints/test_relations.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/test_endpoints/test_relations.py b/tests/test_endpoints/test_relations.py index 9c02b642..72e58a9b 100644 --- a/tests/test_endpoints/test_relations.py +++ b/tests/test_endpoints/test_relations.py @@ -129,12 +129,3 @@ def test_relations_all_endpoint_parms(self): self.assertEqual( result.json[0]["relationship_string"], "second great stepgrandaunt" ) - # check locale parm working - result = self.client.get( - "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?locale" - ) - self.assertEqual(result.status_code, 422) - result = self.client.get( - "/api/relations/9BXKQC1PVLPYFMD6IX/ORFKQC4KLWEGTGR19L/all?locale=1" - ) - self.assertEqual(result.status_code, 200)