From f69e656c336dc3ab977e57fdbb58ac201e9edd73 Mon Sep 17 00:00:00 2001 From: Hilmar Tryggvason Date: Fri, 19 Nov 2021 16:32:39 +0000 Subject: [PATCH 1/2] [friendships] Fix response when creating an existing friendship - Instead of an empty json 200 OK return, 409 CONFLICT - Replace flask.abort with flask_smorest.abort --- driftbase/api/friendships.py | 38 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/driftbase/api/friendships.py b/driftbase/api/friendships.py index 93abe36d..afc79a16 100644 --- a/driftbase/api/friendships.py +++ b/driftbase/api/friendships.py @@ -5,9 +5,9 @@ import marshmallow as ma from drift.core.extensions.jwt import current_user from drift.core.extensions.urlregistry import Endpoints -from flask import request, g, abort, url_for, jsonify +from flask import request, g, url_for, jsonify from flask.views import MethodView -from flask_smorest import Blueprint +from flask_smorest import Blueprint, abort import http.client as http_client from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import aliased @@ -63,7 +63,7 @@ def get(self, player_id): List my friends """ if player_id != current_user["player_id"]: - abort(http_client.FORBIDDEN, description="That is not your player!") + abort(http_client.FORBIDDEN, message="That is not your player!") left = g.db.query(Friendship.id, Friendship.player1_id, Friendship.player2_id).filter_by(player1_id=player_id, status="active") @@ -91,24 +91,24 @@ def post(self, args, player_id): New friend """ if player_id != current_user["player_id"]: - abort(http_client.FORBIDDEN, description="That is not your player!") + abort(http_client.FORBIDDEN, message="That is not your player!") invite_token = args.get("token") # Get the first non-expired invite that matches the token invite = g.db.query(FriendInvite).filter(FriendInvite.token == invite_token, FriendInvite.expiry_date > datetime.datetime.utcnow()).first() if invite is None: - abort(http_client.NOT_FOUND, description="The invite was not found!") + abort(http_client.NOT_FOUND, message="The invite was not found!") if invite.deleted: - abort(http_client.FORBIDDEN, description="The invite has been deleted!") + abort(http_client.FORBIDDEN, message="The invite has been deleted!") friend_id = invite.issued_by_player_id left_id = player_id right_id = friend_id if left_id == right_id: - abort(http_client.FORBIDDEN, description="You cannot befriend yourself!") + abort(http_client.FORBIDDEN, message="You cannot befriend yourself!") if left_id > right_id: left_id, right_id = right_id, left_id @@ -122,7 +122,7 @@ def post(self, args, player_id): if friendship.status == "deleted": friendship.status = "active" else: - return "{}", http_client.OK + abort(http_client.CONFLICT, message="You are already friends with this player!") else: friendship = Friendship(player1_id=left_id, player2_id=right_id) g.db.add(friendship) @@ -153,9 +153,9 @@ def delete(self, friendship_id): friendship = g.db.query(Friendship).filter_by(id=friendship_id).first() if friendship is None: - abort(http_client.NOT_FOUND, description="Unknown friendship") + abort(http_client.NOT_FOUND, message="Unknown friendship") elif friendship.player1_id != player_id and friendship.player2_id != player_id: - abort(http_client.FORBIDDEN, description="You are not friends") + abort(http_client.FORBIDDEN, message="You are not friends") elif friendship.status == "deleted": return jsonify("{}"), http_client.GONE @@ -213,9 +213,9 @@ def post(self, args): valid_message = "no longer valid" if existing_invite.deleted or existing_invite.expiry_date <= datetime.datetime.utcnow() else "valid" log.info(f"Generated duplicate wordlist invite token '{token}'. Existing token is {valid_message}. Re-generating...") else: - abort(http_client.INTERNAL_SERVER_ERROR, description="Could not generate invite token") + abort(http_client.INTERNAL_SERVER_ERROR, message="Could not generate invite token") else: - abort(http_client.BAD_REQUEST, description="Invalid token format") + abort(http_client.BAD_REQUEST, message="Invalid token format") expires_seconds = args.get("expiration_time_seconds") or _get_tenant_config_value("invite_expiration_seconds") expires_seconds = min(expires_seconds, MAX_INVITE_EXPIRATION_SECONDS) @@ -232,7 +232,7 @@ def post(self, args): g.db.add(invite) g.db.commit() except IntegrityError as e: - abort(http_client.BAD_REQUEST, description="Invalid player IDs provided with request.") + abort(http_client.BAD_REQUEST, message="Invalid player IDs provided with request.") if receiving_player_id is not None: self._post_friend_request_message(sending_player_id, receiving_player_id, token, expires_seconds) @@ -248,7 +248,7 @@ def post(self, args): def _validate_friend_request(receiving_player_id): sending_player_id = int(current_user["player_id"]) if receiving_player_id == sending_player_id: - abort(http_client.CONFLICT, description="Cannot send friend requests to yourself") + abort(http_client.CONFLICT, message="Cannot send friend requests to yourself") player1_id, player2_id = min(sending_player_id, receiving_player_id), max(sending_player_id, receiving_player_id) @@ -257,20 +257,20 @@ def _validate_friend_request(receiving_player_id): Friendship.player2_id == player2_id ).first() if existing_friendship and existing_friendship.status == "active": - abort(http_client.CONFLICT, description="You are already friends") # Already friends + abort(http_client.CONFLICT, message="You are already friends") # Already friends pending_invite = g.db.query(FriendInvite). \ filter(FriendInvite.issued_by_player_id == sending_player_id, FriendInvite.issued_to_player_id == receiving_player_id). \ filter(FriendInvite.expiry_date > datetime.datetime.utcnow(), FriendInvite.deleted.is_(False)). \ first() if pending_invite: - abort(http_client.CONFLICT, description="Cannot issue multiple friend requests to the same receiver") + abort(http_client.CONFLICT, message="Cannot issue multiple friend requests to the same receiver") reciprocal_invite = g.db.query(FriendInvite). \ filter(FriendInvite.issued_by_player_id == receiving_player_id, FriendInvite.issued_to_player_id == sending_player_id). \ filter(FriendInvite.expiry_date > datetime.datetime.utcnow(), FriendInvite.deleted.is_(False)).first() if reciprocal_invite: - abort(http_client.CONFLICT, description="The receiver has already sent you a friend request") + abort(http_client.CONFLICT, message="The receiver has already sent you a friend request") @staticmethod def _post_friend_request_message(sender_player_id, receiving_player_id, token, expiry): @@ -294,10 +294,10 @@ def delete(self, invite_id): invite = g.db.query(FriendInvite).filter_by(id=invite_id).first() if not invite: - abort(http_client.NOT_FOUND, description="Invite not found") + abort(http_client.NOT_FOUND, message="Invite not found") elif invite.issued_by_player_id != player_id and invite.issued_to_player_id != player_id: # You may only delete invites sent by you or directly to you. - abort(http_client.FORBIDDEN, description="Not your invite") + abort(http_client.FORBIDDEN, message="Not your invite") elif invite.deleted: return jsonify("{}"), http_client.GONE From f1ab03f14e51976b0046f4dc776df5c8884e6e38 Mon Sep 17 00:00:00 2001 From: Hilmar Tryggvason Date: Fri, 19 Nov 2021 16:38:12 +0000 Subject: [PATCH 2/2] [friendships] Fix adding friend twice test --- driftbase/tests/test_friendships.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driftbase/tests/test_friendships.py b/driftbase/tests/test_friendships.py index 68348ae8..966a557d 100644 --- a/driftbase/tests/test_friendships.py +++ b/driftbase/tests/test_friendships.py @@ -386,7 +386,7 @@ def test_adding_same_friend_twice_changes_nothing(self): # add a friend self.post(self.endpoints["my_friends"], data={"token": token}, expected_status_code=http_client.CREATED) # add same friend again - self.post(self.endpoints["my_friends"], data={"token": token}, expected_status_code=http_client.OK) + self.post(self.endpoints["my_friends"], data={"token": token}, expected_status_code=http_client.CONFLICT) friends = self.get(self.endpoints["my_friends"]).json() self.assertIsInstance(friends, list)