From c95d8420c96afebf569d5bc14ce13bf6623e9a16 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Wed, 4 Nov 2020 20:05:31 -0500 Subject: [PATCH 1/9] Add table metadata --- multinet/api.py | 23 ++++++++++- multinet/db/models/table.py | 61 +++++++++++++++++++++++------- multinet/db/models/workspace.py | 21 +++++++--- multinet/errors.py | 14 ++++++- multinet/swagger/get_metadata.yaml | 20 ++++++++++ multinet/swagger/set_metadata.yaml | 35 +++++++++++++++++ multinet/types.py | 58 +++++++++++++++++++++++++++- 7 files changed, 209 insertions(+), 23 deletions(-) create mode 100644 multinet/swagger/get_metadata.yaml create mode 100644 multinet/swagger/set_metadata.yaml diff --git a/multinet/api.py b/multinet/api.py index c503ccb1..0e16b783 100644 --- a/multinet/api.py +++ b/multinet/api.py @@ -5,7 +5,7 @@ from webargs.flaskparser import use_kwargs from typing import Any, Optional -from multinet.types import EdgeDirection, TableType +from multinet.types import EdgeDirection, UnionTableType from multinet.auth.util import ( require_login, require_reader, @@ -73,7 +73,9 @@ def set_workspace_permissions(workspace: str) -> Any: @require_reader @use_kwargs({"type": fields.Str()}) @swag_from("swagger/workspace_tables.yaml") -def get_workspace_tables(workspace: str, type: TableType = "all") -> Any: # noqa: A002 +def get_workspace_tables( + workspace: str, type: UnionTableType = "all" # noqa: A002 +) -> Any: """Retrieve the tables of a single workspace.""" tables = Workspace(workspace).tables(type) return util.stream(tables) @@ -100,6 +102,23 @@ def get_table_rows(workspace: str, table: str, offset: int = 0, limit: int = 30) return Workspace(workspace).table(table).rows(offset, limit) +@bp.route("/workspaces//tables//metadata", methods=["GET"]) +@require_reader +@swag_from("swagger/get_metadata.yaml") +def get_table_metadata(workspace: str, table: str) -> Any: + """Retrieve the metadata of a table, if it exists.""" + metadata = Workspace(workspace).table(table).get_metadata() + return "" if metadata is None else metadata.dict() + + +@bp.route("/workspaces//tables/
/metadata", methods=["PUT"]) +@require_reader +@swag_from("swagger/set_metadata.yaml") +def set_table_metadata(workspace: str, table: str) -> Any: + """Retrieve the rows and headers of a table.""" + return Workspace(workspace).table(table).set_metadata(request.json).dict() + + @bp.route("/workspaces//graphs", methods=["GET"]) @require_reader @swag_from("swagger/workspace_graphs.yaml") diff --git a/multinet/db/models/table.py b/multinet/db/models/table.py index 751e224e..f1502a36 100644 --- a/multinet/db/models/table.py +++ b/multinet/db/models/table.py @@ -1,11 +1,19 @@ """Operations that deal with tables.""" from __future__ import annotations # noqa: T484 + from arango.collection import StandardCollection from arango.aql import AQL +from pydantic import ValidationError as PydanticValidationError from multinet import util -from multinet.types import EdgeTableProperties -from multinet.errors import ServerError, FlaskTuple +from multinet.db.models import workspace +from multinet.types import ( + EdgeTableProperties, + ArangoEntityDocument, + EntityMetadata, + TableMetadata, +) +from multinet.errors import ServerError, FlaskTuple, InvalidMetadata from typing import List, Set, Dict, Iterable, Union, Optional @@ -25,22 +33,23 @@ def flask_response(self) -> FlaskTuple: class Table: """Tables store tabular data, and are the root of all data storage in Multinet.""" - def __init__(self, name: str, workspace: str, handle: StandardCollection, aql: AQL): + def __init__(self, name: str, workspace: workspace.Workspace): """ Initialize all Table parameters, but make no requests. - The `workspace` parameter is the name of the workspace this table belongs to. - - The `handle` parameter is the handle to the arangodb collection for which this - class instance is associated. - - The `aql` parameter is the AQL handle of the creating Workspace, so that this - class may make AQL requests when necessary. + The `name` parameter is the name of this table. + The `workspace` parameter is the workspace this table belongs to. """ self.name = name - self.workspace = workspace - self.handle = handle - self.aql = aql + + # Used for inserting/modifying table metadata + self.metadata_collection = workspace.entity_metadata_collection() + + # Used for querying table items + self.handle: StandardCollection = workspace.handle.collection(name) + + # Used for running AQL queries when necessary + self.aql: AQL = workspace.handle.aql def rows(self, offset: Optional[int] = None, limit: Optional[int] = None) -> Dict: """Return the desired rows in a table.""" @@ -72,6 +81,32 @@ def headers(self) -> List[str]: return keys + def get_metadata(self) -> Optional[ArangoEntityDocument]: + """Retrieve metadata for this table, if it exists.""" + try: + doc = next(self.metadata_collection.find({"item_id": self.name}, limit=1)) + except StopIteration: + return None + + return ArangoEntityDocument(**doc) + + def set_metadata(self, raw_data: Dict) -> ArangoEntityDocument: + """Set metadata for this table.""" + try: + data = TableMetadata(**raw_data) + except PydanticValidationError: + raise InvalidMetadata(raw_data) + + existing = self.get_metadata() + if existing is not None: + existing.table = data + doc_meta = self.metadata_collection.update(existing.dict()) + else: + entity = EntityMetadata(item_id=self.name, table=data, graph=None) + doc_meta = self.metadata_collection.insert(entity.dict()) + + return ArangoEntityDocument(**self.metadata_collection.get(doc_meta)) + def rename(self, new_name: str) -> None: """Rename a table.""" self.handle.rename(new_name) diff --git a/multinet/db/models/workspace.py b/multinet/db/models/workspace.py index 975906d9..0cbd27d7 100644 --- a/multinet/db/models/workspace.py +++ b/multinet/db/models/workspace.py @@ -5,9 +5,10 @@ from pydantic import BaseModel, Field from arango.exceptions import DatabaseCreateError, EdgeDefinitionCreateError from arango.cursor import Cursor +from arango.collection import StandardCollection from multinet import util -from multinet.types import EdgeTableProperties, TableType +from multinet.types import EdgeTableProperties, UnionTableType from multinet.validation import ValidationFailure, UndefinedTable, UndefinedKeys from multinet.validation.csv import validate_csv from multinet.db import ( @@ -194,6 +195,13 @@ def get_metadata(self) -> Dict: # Copy so modifications to return don't poison cache return copy.deepcopy(doc) + def entity_metadata_collection(self) -> StandardCollection: + """Return the collection handle for table/graph metadata.""" + if not self.readonly_handle.has_collection("_metadata"): + return self.handle.create_collection("_metadata", system=True) + + return self.handle.collection("_metadata") + def graphs(self) -> List[Dict]: """Return the graphs in this workspace.""" return self.readonly_handle.graphs() @@ -267,7 +275,7 @@ def delete_graph(self, name: str) -> bool: return self.handle.delete_graph(name) - def tables(self, table_type: TableType = "all") -> Generator[str, None, None]: + def tables(self, table_type: UnionTableType = "all") -> Generator[str, None, None]: """Return all tables of the specified type.""" def pass_all(x: Dict[str, Any]) -> bool: @@ -299,7 +307,10 @@ def is_node(x: Dict[str, Any]) -> bool: def table(self, name: str) -> Table: """Return a specific table.""" - return Table(name, self.name, self.handle.collection(name), self.handle.aql) + if not self.readonly_handle.has_collection(name): + raise TableNotFound(self.name, name) + + return Table(name, self) def has_table(self, name: str) -> bool: """Return if a specific table exists.""" @@ -307,8 +318,8 @@ def has_table(self, name: str) -> bool: def create_table(self, table: str, edge: bool, sync: bool = False) -> Table: """Create a table in this workspace.""" - table_handle = self.handle.create_collection(table, edge=edge, sync=sync) - return Table(table, self.name, table_handle, self.handle.aql) + self.handle.create_collection(table, edge=edge, sync=sync) + return Table(table, self) def create_aql_table(self, table: str, aql_query: str) -> Table: """Create a table in this workspace from an aql query.""" diff --git a/multinet/errors.py b/multinet/errors.py index bf3ec295..8c1b1a1c 100644 --- a/multinet/errors.py +++ b/multinet/errors.py @@ -1,5 +1,5 @@ """Exception objects representing Multinet-specific HTTP error conditions.""" -from typing import Tuple, Any, Union, List, Sequence +from typing import Tuple, Any, Union, Dict, List, Sequence from typing_extensions import TypedDict from multinet.validation import ValidationFailure @@ -160,6 +160,18 @@ def flask_response(self) -> FlaskTuple: return (self.body, "400 Malformed Request Body") +class InvalidMetadata(ServerError): + """Exception for specifying invalid metadata.""" + + def __init__(self, metadata: Dict): + """Initialize the exception.""" + self.metadata = metadata + + def flask_response(self) -> FlaskTuple: + """Generate a 400 error.""" + return (self.metadata, "400 Invalid Metadata") + + class RequiredParamsMissing(ServerError): """Exception for missing required parameters.""" diff --git a/multinet/swagger/get_metadata.yaml b/multinet/swagger/get_metadata.yaml new file mode 100644 index 00000000..ed34b2c1 --- /dev/null +++ b/multinet/swagger/get_metadata.yaml @@ -0,0 +1,20 @@ +Retrieve the metadata of a table. +--- +parameters: + - $ref: "#/parameters/workspace" + - $ref: "#/parameters/table" + +responses: + 200: + description: The permissions for the given workspace + schema: + $ref: "#/definitions/workspace_permissions" + + 404: + description: Specified workspace or table could not be found + schema: + type: string + example: workspace_or_table_that_doesnt_exist + +tags: + - table diff --git a/multinet/swagger/set_metadata.yaml b/multinet/swagger/set_metadata.yaml new file mode 100644 index 00000000..e91f9421 --- /dev/null +++ b/multinet/swagger/set_metadata.yaml @@ -0,0 +1,35 @@ +Set the metadata of a table. +--- +parameters: + - $ref: "#/parameters/workspace" + - $ref: "#/parameters/table" + - name: metadata + in: body + description: The metadata to set (overwrites existing data) + required: true + schema: + type: object + example: + columns: + - key: test + type: label + + - key: length + type: number + + + +responses: + 200: + description: The permissions for the given workspace + schema: + $ref: "#/definitions/workspace_permissions" + + 404: + description: Specified workspace or table could not be found + schema: + type: string + example: workspace_or_table_that_doesnt_exist + +tags: + - table diff --git a/multinet/types.py b/multinet/types.py index 6159ad6e..8fcd4e59 100644 --- a/multinet/types.py +++ b/multinet/types.py @@ -1,9 +1,63 @@ """Custom types for Multinet codebase.""" -from typing import Dict, Set +from pydantic import BaseModel, Field +from typing import List, Dict, Set, Optional, Any from typing_extensions import Literal, TypedDict EdgeDirection = Literal["all", "incoming", "outgoing"] -TableType = Literal["all", "node", "edge"] + +TableType = Literal["node", "edge"] +UnionTableType = Literal["all", TableType] +ColumnType = Literal["label", "boolean", "category", "number", "date"] + + +class ColumnMetadata(BaseModel): + """Metadata for a table column.""" + + key: str + type: ColumnType + + +class TableMetadata(BaseModel): + """Metadata for a table.""" + + columns: List[ColumnMetadata] = Field(default_factory=list) + + +class GraphMetadata(BaseModel): + """Metadata for a graph.""" + + +class EntityMetadata(BaseModel): + """Metadata for a table or graph.""" + + item_id: str + table: Optional[TableMetadata] + graph: Optional[GraphMetadata] + + +class ArangoEntityDocument(EntityMetadata): + """An entity metadata document with arangodb metadata.""" + + def dict(self, **kwargs: Any) -> Dict: # noqa: A003 + """ + Overload existing dict function to use alias for dict serialization. + + Variable names with leading underscores aren't treated normally, and need to be + aliased to be properly specified. Since pydantic doesn't serialize with alias + names by default, this overload is needed. + """ + + kwargs["by_alias"] = True + return super().dict(**kwargs) + + id: str = Field(alias="_id") + key: str = Field(alias="_key") + rev: str = Field(alias="_rev") + + class Config: + """Model config.""" + + allow_population_by_field_name = True class EdgeTableProperties(TypedDict): From 4b704aef51f80628db799ecb105155b88fd3bd4a Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 5 Nov 2020 14:45:19 -0500 Subject: [PATCH 2/9] Add query param for metadata in csv upload --- multinet/uploaders/csv.py | 14 ++++++++++++-- multinet/uploaders/swagger/csv.yaml | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/multinet/uploaders/csv.py b/multinet/uploaders/csv.py index 9e156fcd..1932643e 100644 --- a/multinet/uploaders/csv.py +++ b/multinet/uploaders/csv.py @@ -1,5 +1,6 @@ """Multinet uploader for CSV files.""" import csv +import json from flasgger import swag_from from io import StringIO @@ -16,7 +17,7 @@ from webargs.flaskparser import use_kwargs # Import types -from typing import Any, List, Dict +from typing import Any, List, Dict, Optional bp = Blueprint("csv", __name__) @@ -47,12 +48,17 @@ def set_table_key(rows: List[Dict[str, str]], key: str) -> List[Dict[str, str]]: { "key": webarg_fields.Str(location="query"), "overwrite": webarg_fields.Bool(location="query"), + "metadata": webarg_fields.Str(location="query"), } ) @require_writer @swag_from("swagger/csv.yaml") def upload( - workspace: str, table: str, key: str = "_key", overwrite: bool = False + workspace: str, + table: str, + key: str = "_key", + overwrite: bool = False, + metadata: Optional[str] = None, ) -> Any: """ Store a CSV file into the database as a node or edge table. @@ -95,6 +101,10 @@ def upload( # Create table and insert the data loaded_table = loaded_workspace.create_table(table, edges) + + if metadata: + loaded_table.set_metadata(json.loads(metadata)) + results = loaded_table.insert(rows) return {"count": len(results)} diff --git a/multinet/uploaders/swagger/csv.yaml b/multinet/uploaders/swagger/csv.yaml index a0eaada2..07c40cc9 100644 --- a/multinet/uploaders/swagger/csv.yaml +++ b/multinet/uploaders/swagger/csv.yaml @@ -34,6 +34,21 @@ parameters: schema: type: boolean default: false + - + name: metadata + in: query + description: Metadata to set for the table + schema: + type: object + example: + columns: + - + key: city + type: label + - + key: size + type: number + responses: 200: From 6ef8fdca1aee350fa18395c398a5f454e5c1ff4e Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Fri, 6 Nov 2020 12:55:15 -0500 Subject: [PATCH 3/9] Add initial metadata tests --- test/test_metadata.py | 70 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 test/test_metadata.py diff --git a/test/test_metadata.py b/test/test_metadata.py new file mode 100644 index 00000000..d7bec316 --- /dev/null +++ b/test/test_metadata.py @@ -0,0 +1,70 @@ +"""Testing operations on metadata.""" +import conftest +import pytest +import json + + +def test_set_valid_table_metadata(populated_workspace, managed_user, server): + """Test that setting valid metadata succeeds.""" + workspace, _, node_table, _ = populated_workspace + metadata = {"columns": [{"key": "test", "type": "label"}]} + + with conftest.login(managed_user, server): + resp = server.put( + f"/api/workspaces/{workspace.name}/tables/{node_table}/metadata", + json=metadata, + ) + + assert resp.status_code == 200 + assert resp.json["table"] == metadata + + +@pytest.mark.parametrize( + "metadata, expected", + [ + ({"columns": [{"key": "test", "type": "invalid"}]}, 400), + ({"columns": [{"foo": "bar"}]}, 400), + ({"foo": "bar"}, 200), + ], +) +def test_set_invalid_table_metadata( + populated_workspace, managed_user, server, metadata, expected +): + """Test that setting invalid metadata fails.""" + workspace, _, node_table, _ = populated_workspace + + with conftest.login(managed_user, server): + resp = server.put( + f"/api/workspaces/{workspace.name}/tables/{node_table}/metadata", + json=metadata, + ) + assert resp.status_code == expected + + +# NOTE: Including metadata in CSV uploads will likely be removed in a future API change +def test_csv_upload_with_metadata( + managed_workspace, managed_user, server, data_directory +): + """Test that uploading a CSV file with metadata succeeds.""" + + table_name = "test" + metadata = {"columns": [{"key": "test", "type": "label"}]} + + with open(data_directory / "membership_with_keys.csv") as csv_file: + request_body = csv_file.read() + + with conftest.login(managed_user, server): + resp = server.post( + f"/api/csv/{managed_workspace.name}/{table_name}", + data=request_body, + query_string={"metadata": json.dumps(metadata)}, + ) + + assert resp.status_code == 200 + + resp = server.get( + f"/api/workspaces/{managed_workspace.name}/tables/{table_name}/metadata" + ) + + assert resp.status_code == 200 + assert resp.json["table"] == metadata From 18832341acca40ff49af148b6721de08c0172786 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 9 Nov 2020 12:29:14 -0500 Subject: [PATCH 4/9] Revert `TableType` change --- multinet/api.py | 6 ++---- multinet/db/models/workspace.py | 4 ++-- multinet/types.py | 3 +-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/multinet/api.py b/multinet/api.py index 0e16b783..ce953f0a 100644 --- a/multinet/api.py +++ b/multinet/api.py @@ -5,7 +5,7 @@ from webargs.flaskparser import use_kwargs from typing import Any, Optional -from multinet.types import EdgeDirection, UnionTableType +from multinet.types import EdgeDirection, TableType from multinet.auth.util import ( require_login, require_reader, @@ -73,9 +73,7 @@ def set_workspace_permissions(workspace: str) -> Any: @require_reader @use_kwargs({"type": fields.Str()}) @swag_from("swagger/workspace_tables.yaml") -def get_workspace_tables( - workspace: str, type: UnionTableType = "all" # noqa: A002 -) -> Any: +def get_workspace_tables(workspace: str, type: TableType = "all") -> Any: # noqa: A002 """Retrieve the tables of a single workspace.""" tables = Workspace(workspace).tables(type) return util.stream(tables) diff --git a/multinet/db/models/workspace.py b/multinet/db/models/workspace.py index 0cbd27d7..e9e100c0 100644 --- a/multinet/db/models/workspace.py +++ b/multinet/db/models/workspace.py @@ -8,7 +8,7 @@ from arango.collection import StandardCollection from multinet import util -from multinet.types import EdgeTableProperties, UnionTableType +from multinet.types import EdgeTableProperties, TableType from multinet.validation import ValidationFailure, UndefinedTable, UndefinedKeys from multinet.validation.csv import validate_csv from multinet.db import ( @@ -275,7 +275,7 @@ def delete_graph(self, name: str) -> bool: return self.handle.delete_graph(name) - def tables(self, table_type: UnionTableType = "all") -> Generator[str, None, None]: + def tables(self, table_type: TableType = "all") -> Generator[str, None, None]: """Return all tables of the specified type.""" def pass_all(x: Dict[str, Any]) -> bool: diff --git a/multinet/types.py b/multinet/types.py index 8fcd4e59..1bb7ec6f 100644 --- a/multinet/types.py +++ b/multinet/types.py @@ -5,8 +5,7 @@ EdgeDirection = Literal["all", "incoming", "outgoing"] -TableType = Literal["node", "edge"] -UnionTableType = Literal["all", TableType] +TableType = Literal["all", "node", "edge"] ColumnType = Literal["label", "boolean", "category", "number", "date"] From d54d082cf0d9c50f10fcd3786bdf693f1b7d64ed Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 9 Nov 2020 12:52:51 -0500 Subject: [PATCH 5/9] Update `get_metadata` to always return doc --- multinet/api.py | 3 +-- multinet/db/models/table.py | 8 ++++++-- mypy_stubs/arango/collection.pyi | 1 - 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/multinet/api.py b/multinet/api.py index ce953f0a..88c550a9 100644 --- a/multinet/api.py +++ b/multinet/api.py @@ -105,8 +105,7 @@ def get_table_rows(workspace: str, table: str, offset: int = 0, limit: int = 30) @swag_from("swagger/get_metadata.yaml") def get_table_metadata(workspace: str, table: str) -> Any: """Retrieve the metadata of a table, if it exists.""" - metadata = Workspace(workspace).table(table).get_metadata() - return "" if metadata is None else metadata.dict() + return Workspace(workspace).table(table).get_metadata().dict() @bp.route("/workspaces//tables/
/metadata", methods=["PUT"]) diff --git a/multinet/db/models/table.py b/multinet/db/models/table.py index f1502a36..7fb1a062 100644 --- a/multinet/db/models/table.py +++ b/multinet/db/models/table.py @@ -81,12 +81,16 @@ def headers(self) -> List[str]: return keys - def get_metadata(self) -> Optional[ArangoEntityDocument]: + def get_metadata(self) -> ArangoEntityDocument: """Retrieve metadata for this table, if it exists.""" try: doc = next(self.metadata_collection.find({"item_id": self.name}, limit=1)) except StopIteration: - return None + entity = EntityMetadata(item_id=self.name, table=TableMetadata()) + + # Return is just metadata, merge with entity to get full doc + doc = self.metadata_collection.insert(entity.dict()) + doc.update(entity.dict()) return ArangoEntityDocument(**doc) diff --git a/mypy_stubs/arango/collection.pyi b/mypy_stubs/arango/collection.pyi index a8d905eb..6599fd0c 100644 --- a/mypy_stubs/arango/collection.pyi +++ b/mypy_stubs/arango/collection.pyi @@ -20,7 +20,6 @@ class Collection: class StandardCollection(Collection): name: str - def insert( self, document: Any, From 611816475e1a4549317da9ad8d7ae8527f3a5e62 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 9 Nov 2020 12:53:19 -0500 Subject: [PATCH 6/9] Update `set_metadata` with cleaner syntax --- multinet/db/models/table.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/multinet/db/models/table.py b/multinet/db/models/table.py index 7fb1a062..1f5778fa 100644 --- a/multinet/db/models/table.py +++ b/multinet/db/models/table.py @@ -101,15 +101,13 @@ def set_metadata(self, raw_data: Dict) -> ArangoEntityDocument: except PydanticValidationError: raise InvalidMetadata(raw_data) - existing = self.get_metadata() - if existing is not None: - existing.table = data - doc_meta = self.metadata_collection.update(existing.dict()) - else: - entity = EntityMetadata(item_id=self.name, table=data, graph=None) - doc_meta = self.metadata_collection.insert(entity.dict()) - - return ArangoEntityDocument(**self.metadata_collection.get(doc_meta)) + entity = self.get_metadata() + entity.table = data + + new_doc = entity.dict() + new_doc.update(self.metadata_collection.insert(new_doc, overwrite=True)) + + return ArangoEntityDocument(**new_doc) def rename(self, new_name: str) -> None: """Rename a table.""" From 2cbfec056b4a2e67cc722b8f76c53ea0e6bce3fa Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 9 Nov 2020 12:55:17 -0500 Subject: [PATCH 7/9] Remove excess blank lines --- multinet/swagger/set_metadata.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/multinet/swagger/set_metadata.yaml b/multinet/swagger/set_metadata.yaml index e91f9421..904e951b 100644 --- a/multinet/swagger/set_metadata.yaml +++ b/multinet/swagger/set_metadata.yaml @@ -17,8 +17,6 @@ parameters: - key: length type: number - - responses: 200: description: The permissions for the given workspace From 5c168f093475763f194bc179df213268faeabf3e Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 9 Nov 2020 13:22:45 -0500 Subject: [PATCH 8/9] Throw error for invalid csv metadata arg This also updates the BadQueryArgument error, as it previously included an "allowed" field, which may not always apply. --- multinet/api.py | 2 +- multinet/errors.py | 12 ++---------- multinet/uploaders/csv.py | 8 +++++--- test/test_metadata.py | 24 +++++++++++++++++++++++- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/multinet/api.py b/multinet/api.py index 88c550a9..3229be9a 100644 --- a/multinet/api.py +++ b/multinet/api.py @@ -174,7 +174,7 @@ def get_node_edges( """Return the edges connected to a node.""" allowed = ["incoming", "outgoing", "all"] if direction not in allowed: - raise BadQueryArgument("direction", direction, allowed) + raise BadQueryArgument("direction", direction) return ( Workspace(workspace) diff --git a/multinet/errors.py b/multinet/errors.py index 8c1b1a1c..c40fda06 100644 --- a/multinet/errors.py +++ b/multinet/errors.py @@ -1,12 +1,10 @@ """Exception objects representing Multinet-specific HTTP error conditions.""" from typing import Tuple, Any, Union, Dict, List, Sequence -from typing_extensions import TypedDict from multinet.validation import ValidationFailure FlaskTuple = Tuple[Any, Union[int, str]] -Payload = TypedDict("Payload", {"argument": str, "value": str, "allowed": List[str]}) class ServerError(Exception): @@ -118,20 +116,14 @@ def __init__(self, table: str, node: str): class BadQueryArgument(ServerError): """Exception for illegal query argument value.""" - def __init__(self, argument: str, value: str, allowed: List[str]): + def __init__(self, argument: str, value: str): """Initialize the exception.""" self.argument = argument self.value = value - self.allowed = allowed def flask_response(self) -> FlaskTuple: """Generate a 400 error for the bad argument.""" - payload: Payload = { - "argument": self.argument, - "value": self.value, - "allowed": self.allowed, - } - + payload = {"argument": self.argument, "value": self.value} return (payload, "400 Bad Query Argument") diff --git a/multinet/uploaders/csv.py b/multinet/uploaders/csv.py index 1932643e..5f83a74f 100644 --- a/multinet/uploaders/csv.py +++ b/multinet/uploaders/csv.py @@ -7,7 +7,7 @@ from multinet import util from multinet.db.models.workspace import Workspace from multinet.auth.util import require_writer -from multinet.errors import AlreadyExists, FlaskTuple, ServerError +from multinet.errors import AlreadyExists, FlaskTuple, ServerError, BadQueryArgument from multinet.util import decode_data from multinet.validation.csv import validate_csv @@ -103,8 +103,10 @@ def upload( loaded_table = loaded_workspace.create_table(table, edges) if metadata: - loaded_table.set_metadata(json.loads(metadata)) + try: + loaded_table.set_metadata(json.loads(metadata)) + except json.decoder.JSONDecodeError: + raise BadQueryArgument("metadata", metadata) results = loaded_table.insert(rows) - return {"count": len(results)} diff --git a/test/test_metadata.py b/test/test_metadata.py index d7bec316..03bcef19 100644 --- a/test/test_metadata.py +++ b/test/test_metadata.py @@ -42,7 +42,7 @@ def test_set_invalid_table_metadata( # NOTE: Including metadata in CSV uploads will likely be removed in a future API change -def test_csv_upload_with_metadata( +def test_csv_upload_with_valid_metadata( managed_workspace, managed_user, server, data_directory ): """Test that uploading a CSV file with metadata succeeds.""" @@ -68,3 +68,25 @@ def test_csv_upload_with_metadata( assert resp.status_code == 200 assert resp.json["table"] == metadata + + +def test_csv_upload_with_invalid_metadata( + managed_workspace, managed_user, server, data_directory +): + """Test that uploading a CSV file with invalid json fails properly.""" + + table_name = "test" + metadata_str = "{" + with open(data_directory / "membership_with_keys.csv") as csv_file: + request_body = csv_file.read() + + with conftest.login(managed_user, server): + resp = server.post( + f"/api/csv/{managed_workspace.name}/{table_name}", + data=request_body, + query_string={"metadata": metadata_str}, + ) + + assert resp.status_code == 400 + assert resp.json["argument"] == "metadata" + assert resp.json["value"] == metadata_str From 70bfc39b9fd0f52dc1899952097b5b69b2d89217 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Mon, 9 Nov 2020 13:26:31 -0500 Subject: [PATCH 9/9] Remove incorrect swagger doc defs --- multinet/swagger/get_metadata.yaml | 4 +--- multinet/swagger/set_metadata.yaml | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/multinet/swagger/get_metadata.yaml b/multinet/swagger/get_metadata.yaml index ed34b2c1..4f6810a0 100644 --- a/multinet/swagger/get_metadata.yaml +++ b/multinet/swagger/get_metadata.yaml @@ -6,9 +6,7 @@ parameters: responses: 200: - description: The permissions for the given workspace - schema: - $ref: "#/definitions/workspace_permissions" + description: The metadata for this table 404: description: Specified workspace or table could not be found diff --git a/multinet/swagger/set_metadata.yaml b/multinet/swagger/set_metadata.yaml index 904e951b..870db626 100644 --- a/multinet/swagger/set_metadata.yaml +++ b/multinet/swagger/set_metadata.yaml @@ -19,9 +19,7 @@ parameters: responses: 200: - description: The permissions for the given workspace - schema: - $ref: "#/definitions/workspace_permissions" + description: The metadata for this table 404: description: Specified workspace or table could not be found