Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2714 delete cases #2753

Merged
merged 5 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,38 @@ def batch_update_query(self, query: str, update: dict) -> int:
updates.append(an_update)
return self.batch_update(updates)

def delete_case(self, case_id: str):
"""Remove a case. Raises NotFoundError if no case with the given id exists."""
if self.store.case_by_id(case_id) is None:
raise NotFoundError(f"No case with ID {case_id}")
self.store.delete_case(case_id)

def batch_delete(
self,
query: Optional[str] = None,
case_ids: Optional[list[str]] = None,
threshold: Optional[int] = None,
):
if not ((query is None) ^ (case_ids is None)):
raise PreconditionUnsatisfiedError(
"Must specify exactly one of query or case ID list"
)
if case_ids is not None:
for case_id in case_ids:
self.delete_case(case_id)
else: # query is not None
filter = self.parse_filter(query)
if filter is None or filter.matches_everything():
raise PreconditionUnsatisfiedError(
f"unspported query in batch_delete: {query}"
)
target_case_count = self.store.count_cases(filter)
if threshold and target_case_count > threshold:
raise ValidationError(
f"Command would delete {target_case_count} cases, above threshold of {threshold}"
)
self.store.delete_cases(filter)

def validate_updated_case(self, id: str, update: DocumentUpdate):
"""Find out whether updating a case would result in it being invalid.
Raises NotFoundError if the case doesn't exist, or ValidationError if
Expand Down Expand Up @@ -279,17 +311,20 @@ def check_case_preconditions(self, case: Case):
@staticmethod
def parse_filter(filter: str) -> Filter:
"""Interpret the filter query in the incoming request."""
if filter is None:
if filter is None or len(filter) == 0:
return Anything()
# split query on spaces
components = filter.split(" ")
filters = [CaseController.individual_filter(c) for c in components]
if None in filters:
return None
if len(filters) == 1:
return filters[0]
else:
return AndFilter(filters)
try:
# split query on spaces
components = filter.split(" ")
filters = [CaseController.individual_filter(c) for c in components]
if None in filters:
return None
if len(filters) == 1:
return filters[0]
else:
return AndFilter(filters)
except ValueError:
raise PreconditionUnsatisfiedError(f"Malformed query {filter}")

@staticmethod
def individual_filter(term: str) -> Filter:
Expand Down
20 changes: 16 additions & 4 deletions data-serving/reusable-data-service/data_service/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,21 @@
case_controller = None # Will be set up in main()


@app.route("/api/cases/<id>", methods=["GET", "PUT"])
@app.route("/api/cases/<id>", methods=["GET", "PUT", "DELETE"])
def get_case(id):
try:
if request.method == "GET":
return jsonify(case_controller.get_case(id)), 200
else:
elif request.method == "PUT":
return jsonify(case_controller.update_case(id, request.get_json())), 200
else:
case_controller.delete_case(id)
return "", 204
except WebApplicationError as e:
return jsonify({"message": e.args[0]}), e.http_code


@app.route("/api/cases", methods=["POST", "GET"])
@app.route("/api/cases", methods=["POST", "GET", "DELETE"])
def list_cases():
if request.method == "GET":
page = request.args.get("page", type=int)
Expand All @@ -45,7 +48,7 @@ def list_cases():
)
except WebApplicationError as e:
return jsonify({"message": e.args[0]}), e.http_code
else:
elif request.method == "POST":
potential_case = request.get_json()
validate_only = request.args.get("validate_only", type=bool)
if validate_only:
Expand All @@ -62,6 +65,15 @@ def list_cases():
return "", 201
except WebApplicationError as e:
return jsonify({"message": e.args[0]}), e.http_code
else:
req = request.get_json()
try:
case_controller.batch_delete(
req.get("query"), req.get("caseIds"), req.get("maxCasesThreshold")
)
return "", 204
except WebApplicationError as e:
return jsonify({"message": e.args[0]}), e.http_code


@app.route("/api/cases/batchUpsert", methods=["POST"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
class Filter:
"""Represents any filter on a collection."""

pass
def matches_everything(self) -> bool:
"""Indicate whether this filter is satisfied for all input."""
return False


class Anything(Filter):
Expand All @@ -13,6 +15,9 @@ class Anything(Filter):
def __str__(self) -> str:
return "Anything()"

def matches_everything(self) -> bool:
return True


class PropertyFilter(Filter):
"""Represents a test that an object's property has a value that satisfies some constraint."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,15 @@ def batch_update(self, updates: dict[str, DocumentUpdate]):
result = self.get_case_collection().bulk_write(update_ones)
return result.modified_count

def delete_case(self, id: str):
"""Delete the case with the specified ID"""
self.get_case_collection().delete_one({"_id": ObjectId(id)})

def delete_cases(self, filter: Filter):
"""Delete all cases that match the specified filter."""
predicate = filter.to_mongo_query()
self.get_case_collection().delete_many(predicate)

@staticmethod
def mongodb_update_command(update: DocumentUpdate):
objectify_id = (
Expand Down
86 changes: 86 additions & 0 deletions data-serving/reusable-data-service/tests/test_case_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ def excluded_cases(self, source_id: str, filter: Optional[str] = None):
and c.caseReference.status == "EXCLUDED"
]

def delete_case(self, source_id: str):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be delete_source?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh no, it should be case_id 🤦🏻

del self.cases[source_id]

def delete_cases(self, query):
self.cases = dict()

def matching_case_iterator(self, query):
return iter(self.cases.values())

Expand Down Expand Up @@ -599,3 +605,83 @@ def test_batch_update_query_returns_modified_count(case_controller):
query = None # didn't implement rich queries on the test store
modified = case_controller.batch_update_query(query, update)
assert modified == 4


def test_delete_present_case_deletes_case(case_controller):
for i in range(4):
_ = case_controller.create_case(
{
"confirmationDate": date(2021, 6, i + 1),
"caseReference": {"sourceId": "123ab4567890123ef4567890"},
},
)
case_controller.delete_case("1")
assert case_controller.store.count_cases() == 3
assert case_controller.store.case_by_id("1") is None


def test_delete_absent_case_raises_NotFoundError(case_controller):
with pytest.raises(NotFoundError):
case_controller.delete_case("1")


def test_cannot_batch_delete_both_query_and_case_ids(case_controller):
with pytest.raises(PreconditionUnsatisfiedError):
case_controller.batch_delete(query="", case_ids=[])


def test_cannot_batch_delete_neither_query_nor_case_ids(case_controller):
with pytest.raises(PreconditionUnsatisfiedError):
case_controller.batch_delete(None, None)


def test_cannot_batch_delete_everything(case_controller):
with pytest.raises(PreconditionUnsatisfiedError):
case_controller.batch_delete("", None)


def test_cannot_batch_delete_with_malformed_query(case_controller):
with pytest.raises(PreconditionUnsatisfiedError):
case_controller.batch_delete(" ", None)


def test_cannot_batch_delete_more_cases_than_threshold(case_controller):
for i in range(4):
_ = case_controller.create_case(
{
"confirmationDate": date(2021, 6, i + 1),
"caseReference": {"sourceId": "123ab4567890123ef4567890"},
},
)
with pytest.raises(ValidationError):
case_controller.batch_delete("dateconfirmedafter:2021-05-02", None, 1)
assert case_controller.store.count_cases() == 4


def test_batch_delete_with_case_ids(case_controller):
for i in range(4):
_ = case_controller.create_case(
{
"confirmationDate": date(2021, 6, i + 1),
"caseReference": {"sourceId": "123ab4567890123ef4567890"},
},
)
case_controller.batch_delete(None, ["1", "2"])
assert case_controller.store.count_cases() == 2


def test_batch_delete_with_query(case_controller):
"""This test is deliberately set up to effectively delete all cases because
anything more nuanced would require interpreting filter logic in the test store,
which is a lot of complexity for little value. Look to the end-to-end tests for
better tests of the filtering logic, because the filters should definitely work in
production data stores!"""
for i in range(4):
_ = case_controller.create_case(
{
"confirmationDate": date(2021, 6, i + 1),
"caseReference": {"sourceId": "123ab4567890123ef4567890"},
},
)
case_controller.batch_delete("dateconfirmedafter:2021-05-02", None)
assert case_controller.store.count_cases() == 0
76 changes: 76 additions & 0 deletions data-serving/reusable-data-service/tests/test_case_end_to_end.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,3 +527,79 @@ def test_batch_update_query(client_with_patched_mongo):
)
assert post_result.status_code == 200
assert post_result.get_json()["numModified"] == 2


def test_delete_case(client_with_patched_mongo):
db = pymongo.MongoClient("mongodb://localhost:27017/outbreak")
collection = db["outbreak"]["cases"]
inserted = collection.insert_one(
{
"confirmationDate": datetime(2022, 5, 10),
"caseReference": {"sourceId": bson.ObjectId("fedc12345678901234567890")},
}
).inserted_id
delete_result = client_with_patched_mongo.delete(f"/api/cases/{str(inserted)}")
assert delete_result.status_code == 204
assert collection.count_documents({}) == 0


def test_delete_case_404_on_wrong_id(client_with_patched_mongo):
id = str(bson.ObjectId())
delete_result = client_with_patched_mongo.delete(f"/api/cases/{id}")
assert delete_result.status_code == 404


def test_batch_delete_with_ids(client_with_patched_mongo):
db = pymongo.MongoClient("mongodb://localhost:27017/outbreak")
inserted = db["outbreak"]["cases"].insert_many(
[
{
"confirmationDate": datetime(2022, 5, i),
"caseReference": {
"sourceId": bson.ObjectId("fedc12345678901234567890"),
"status": "EXCLUDED",
},
"caseExclusion": {
"date": datetime(2022, 6, i),
"note": f"Excluded upon this day, the {i}th of June",
},
}
for i in range(1, 4)
]
)
inserted_ids = [str(anId) for anId in inserted.inserted_ids]
delete_result = client_with_patched_mongo.delete(
"/api/cases", json={"caseIds": [inserted_ids[1], inserted_ids[2]]}
)
assert delete_result.status_code == 204
for i in range(len(inserted_ids)):
get_result = client_with_patched_mongo.get(f"/api/cases/{inserted_ids[i]}")
assert get_result.status_code == 200 if i == 0 else 404


def test_batch_delete_with_query(client_with_patched_mongo):
db = pymongo.MongoClient("mongodb://localhost:27017/outbreak")
inserted = db["outbreak"]["cases"].insert_many(
[
{
"confirmationDate": datetime(2022, 5, i),
"caseReference": {
"sourceId": bson.ObjectId("fedc12345678901234567890"),
"status": "EXCLUDED",
},
"caseExclusion": {
"date": datetime(2022, 6, i),
"note": f"Excluded upon this day, the {i}th of June",
},
}
for i in range(1, 4)
]
)
inserted_ids = [str(anId) for anId in inserted.inserted_ids]
delete_result = client_with_patched_mongo.delete(
"/api/cases", json={"query": "dateconfirmedafter:2022-05-01"}
)
assert delete_result.status_code == 204
for i in range(len(inserted_ids)):
get_result = client_with_patched_mongo.get(f"/api/cases/{inserted_ids[i]}")
assert get_result.status_code == 200 if i == 0 else 404