From 663c2be9d918551aee51afbdf08ad8c048c7293f Mon Sep 17 00:00:00 2001 From: kompotkot Date: Thu, 10 Aug 2023 10:40:46 +0000 Subject: [PATCH 1/4] Request ID for call requests migration --- .../040f2dfde5a5_request_id_bytes_column.py | 75 +++++++++++++++++++ engineapi/engineapi/models.py | 9 ++- 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py diff --git a/engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py b/engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py new file mode 100644 index 00000000..0f038f1e --- /dev/null +++ b/engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py @@ -0,0 +1,75 @@ +"""Request ID bytes column + +Revision ID: 040f2dfde5a5 +Revises: b4257b10daaf +Create Date: 2023-08-10 08:58:22.052336 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '040f2dfde5a5' +down_revision = 'b4257b10daaf' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('call_requests', sa.Column('request_id', sa.BigInteger(), nullable=True)) + op.create_index(op.f('ix_call_requests_request_id'), 'call_requests', ['request_id'], unique=False) + op.create_unique_constraint(op.f('uq_call_requests_registered_contract_id'), 'call_requests', ['registered_contract_id', 'request_id']) + + # Manual + # Fetch IDs of duplicates for 'dropper-v0.2.0' call_request_type and delete it + op.execute("""WITH Duplicates AS ( + SELECT + id, + registered_contract_id, + call_request_type_name, + parameters->'requestID' AS request_id, + created_at, + ROW_NUMBER() OVER ( + PARTITION BY + registered_contract_id, + call_request_type_name, + parameters->'requestID' + ORDER BY created_at ASC + ) AS row_num + FROM call_requests + WHERE call_request_type_name = 'dropper-v0.2.0' +), +DeleteDuplicates AS ( +SELECT id +FROM + Duplicates +WHERE + row_num < ( + SELECT COUNT(*) FROM Duplicates d2 + WHERE d2.registered_contract_id = Duplicates.registered_contract_id + AND d2.call_request_type_name = Duplicates.call_request_type_name + AND d2.request_id = Duplicates.request_id + ) +) +DELETE FROM call_requests WHERE id IN (SELECT id FROM DeleteDuplicates);""") + + # Fulfill not empty requestID values + op.execute("UPDATE call_requests SET request_id = CAST(parameters->>'requestID' AS bigint) WHERE parameters->>'requestID' IS NOT NULL;") + # Fulfill raw types with random requestID + op.execute("UPDATE call_requests SET request_id = FLOOR(RANDOM()* 120500600 + 120400600) WHERE parameters->>'requestID' IS NULL;") + op.alter_column("call_requests", "request_id", nullable=False) + + # Other + op.create_unique_constraint(op.f('uq_blockchains_id'), 'blockchains', ['id']) + op.create_unique_constraint(op.f('uq_call_request_types_name'), 'call_request_types', ['name']) + op.create_unique_constraint(op.f('uq_metatx_requesters_id'), 'metatx_requesters', ['id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_call_requests_request_id'), table_name='call_requests') + op.drop_column('call_requests', 'request_id') + # ### end Alembic commands ### diff --git a/engineapi/engineapi/models.py b/engineapi/engineapi/models.py index 895cb136..fa9d5743 100644 --- a/engineapi/engineapi/models.py +++ b/engineapi/engineapi/models.py @@ -13,7 +13,7 @@ from sqlalchemy import ( UniqueConstraint, Integer, ) -from sqlalchemy.dialects.postgresql import JSONB, UUID, ARRAY +from sqlalchemy.dialects.postgresql import JSONB, UUID from sqlalchemy.ext.compiler import compiles from sqlalchemy.orm import relationship from sqlalchemy.ext.declarative import declarative_base @@ -278,6 +278,12 @@ class RegisteredContract(Base): # type: ignore class CallRequest(Base): __tablename__ = "call_requests" + __table_args__ = ( + UniqueConstraint( + "registered_contract_id", + "request_id", + ), + ) id = Column( UUID(as_uuid=True), @@ -304,6 +310,7 @@ class CallRequest(Base): caller = Column(VARCHAR(256), nullable=False, index=True) method = Column(String, nullable=False, index=True) + request_id = Column(BigInteger, nullable=False, index=True) # TODO(zomglings): Should we conditional indices on parameters depending on the contract type? parameters = Column(JSONB, nullable=False) From f1a84d8cec27decec0a29ecc4aab9e036c19ce28 Mon Sep 17 00:00:00 2001 From: kompotkot Date: Thu, 10 Aug 2023 10:54:53 +0000 Subject: [PATCH 2/4] Metatx routes fix to work with call requests --- engineapi/engineapi/contracts_actions.py | 12 +++++++++++- engineapi/engineapi/data.py | 2 ++ engineapi/engineapi/routes/metatx.py | 5 +++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/engineapi/engineapi/contracts_actions.py b/engineapi/engineapi/contracts_actions.py index e3039ff0..8d9752cd 100644 --- a/engineapi/engineapi/contracts_actions.py +++ b/engineapi/engineapi/contracts_actions.py @@ -65,6 +65,12 @@ class ContractAlreadyRegistered(Exception): pass +class CallRequestAlreadyRegistered(Exception): + """ + Raised when call request with same parameters registered. + """ + + def parse_registered_contract_response( obj: Tuple[RegisteredContract, Blockchain] ) -> data.RegisteredContractResponse: @@ -92,6 +98,7 @@ def parse_call_request_response( call_request_type=obj[0].call_request_type_name, caller=obj[0].caller, method=obj[0].method, + request_id=obj[0].request_id, parameters=obj[0].parameters, expires_at=obj[0].expires_at, created_at=obj[0].created_at, @@ -112,7 +119,6 @@ def validate_method_and_params( ) required_params = { "dropId", - "requestID", "blockDeadline", "amount", "signer", @@ -397,6 +403,7 @@ def request_calls( metatx_requester_id=metatx_requester_id, caller=normalized_caller, method=specification.method, + request_id=specification.request_id, parameters=specification.parameters, expires_at=expires_at, ) @@ -405,6 +412,9 @@ def request_calls( # Insert the new rows into the database in a single transaction try: db_session.commit() + except IntegrityError as err: + db_session.rollback() + raise CallRequestAlreadyRegistered() except Exception as e: db_session.rollback() raise e diff --git a/engineapi/engineapi/data.py b/engineapi/engineapi/data.py index fe90dbf0..4e87ea8d 100644 --- a/engineapi/engineapi/data.py +++ b/engineapi/engineapi/data.py @@ -271,6 +271,7 @@ class CallSpecification(BaseModel): caller: str method: str call_request_type: str = "dropper-v0.2.0" + request_id: int parameters: Dict[str, Any] @validator("caller") @@ -302,6 +303,7 @@ class CallRequestResponse(BaseModel): call_request_type: Optional[str] = None caller: str method: str + request_id: int parameters: Dict[str, Any] expires_at: Optional[datetime] = None created_at: datetime diff --git a/engineapi/engineapi/routes/metatx.py b/engineapi/engineapi/routes/metatx.py index 1837a339..6ba090b1 100644 --- a/engineapi/engineapi/routes/metatx.py +++ b/engineapi/engineapi/routes/metatx.py @@ -382,6 +382,11 @@ async def create_requests( status_code=400, detail=f"Unacceptable call request required params specified, err: {err}", ) + except contracts_actions.CallRequestAlreadyRegistered: + raise EngineHTTPException( + status_code=409, + detail="Call request with same parameters registered", + ) except Exception as err: logger.error(repr(err)) raise EngineHTTPException(status_code=500) From 5e034737055be25e09c69f2d88b18397e9a5c111 Mon Sep 17 00:00:00 2001 From: kompotkot Date: Thu, 10 Aug 2023 15:42:08 +0000 Subject: [PATCH 3/4] Switched call requests request id column to decimals --- ....py => 040f2dfde5a5_request_id_decimal_column.py} | 12 ++++++------ engineapi/engineapi/contracts_actions.py | 2 +- engineapi/engineapi/data.py | 4 ++-- engineapi/engineapi/models.py | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) rename engineapi/alembic/versions/{040f2dfde5a5_request_id_bytes_column.py => 040f2dfde5a5_request_id_decimal_column.py} (82%) diff --git a/engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py b/engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py similarity index 82% rename from engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py rename to engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py index 0f038f1e..f9315a8b 100644 --- a/engineapi/alembic/versions/040f2dfde5a5_request_id_bytes_column.py +++ b/engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py @@ -1,4 +1,4 @@ -"""Request ID bytes column +"""Request ID decimal column Revision ID: 040f2dfde5a5 Revises: b4257b10daaf @@ -18,7 +18,7 @@ depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column('call_requests', sa.Column('request_id', sa.BigInteger(), nullable=True)) + op.add_column('call_requests', sa.Column('request_id', sa.DECIMAL(), nullable=True)) op.create_index(op.f('ix_call_requests_request_id'), 'call_requests', ['request_id'], unique=False) op.create_unique_constraint(op.f('uq_call_requests_registered_contract_id'), 'call_requests', ['registered_contract_id', 'request_id']) @@ -56,15 +56,15 @@ WHERE DELETE FROM call_requests WHERE id IN (SELECT id FROM DeleteDuplicates);""") # Fulfill not empty requestID values - op.execute("UPDATE call_requests SET request_id = CAST(parameters->>'requestID' AS bigint) WHERE parameters->>'requestID' IS NOT NULL;") + op.execute("UPDATE call_requests SET request_id = CAST(parameters->>'requestID' AS DECIMAL) WHERE parameters->>'requestID' IS NOT NULL;") # Fulfill raw types with random requestID op.execute("UPDATE call_requests SET request_id = FLOOR(RANDOM()* 120500600 + 120400600) WHERE parameters->>'requestID' IS NULL;") op.alter_column("call_requests", "request_id", nullable=False) # Other - op.create_unique_constraint(op.f('uq_blockchains_id'), 'blockchains', ['id']) - op.create_unique_constraint(op.f('uq_call_request_types_name'), 'call_request_types', ['name']) - op.create_unique_constraint(op.f('uq_metatx_requesters_id'), 'metatx_requesters', ['id']) + # op.create_unique_constraint(op.f('uq_blockchains_id'), 'blockchains', ['id']) + # op.create_unique_constraint(op.f('uq_call_request_types_name'), 'call_request_types', ['name']) + # op.create_unique_constraint(op.f('uq_metatx_requesters_id'), 'metatx_requesters', ['id']) # ### end Alembic commands ### diff --git a/engineapi/engineapi/contracts_actions.py b/engineapi/engineapi/contracts_actions.py index 8d9752cd..e5a632c1 100644 --- a/engineapi/engineapi/contracts_actions.py +++ b/engineapi/engineapi/contracts_actions.py @@ -98,7 +98,7 @@ def parse_call_request_response( call_request_type=obj[0].call_request_type_name, caller=obj[0].caller, method=obj[0].method, - request_id=obj[0].request_id, + request_id=str(obj[0].request_id), parameters=obj[0].parameters, expires_at=obj[0].expires_at, created_at=obj[0].created_at, diff --git a/engineapi/engineapi/data.py b/engineapi/engineapi/data.py index 4e87ea8d..3c7bdd48 100644 --- a/engineapi/engineapi/data.py +++ b/engineapi/engineapi/data.py @@ -271,7 +271,7 @@ class CallSpecification(BaseModel): caller: str method: str call_request_type: str = "dropper-v0.2.0" - request_id: int + request_id: str parameters: Dict[str, Any] @validator("caller") @@ -303,7 +303,7 @@ class CallRequestResponse(BaseModel): call_request_type: Optional[str] = None caller: str method: str - request_id: int + request_id: str parameters: Dict[str, Any] expires_at: Optional[datetime] = None created_at: datetime diff --git a/engineapi/engineapi/models.py b/engineapi/engineapi/models.py index fa9d5743..36cc975d 100644 --- a/engineapi/engineapi/models.py +++ b/engineapi/engineapi/models.py @@ -1,6 +1,7 @@ import uuid from sqlalchemy import ( + DECIMAL, VARCHAR, BigInteger, Boolean, @@ -8,15 +9,15 @@ from sqlalchemy import ( DateTime, ForeignKey, Index, + Integer, MetaData, String, UniqueConstraint, - Integer, ) from sqlalchemy.dialects.postgresql import JSONB, UUID from sqlalchemy.ext.compiler import compiles -from sqlalchemy.orm import relationship from sqlalchemy.ext.declarative import declarative_base +from sqlalchemy.orm import relationship from sqlalchemy.sql import and_, expression """ @@ -310,8 +311,7 @@ class CallRequest(Base): caller = Column(VARCHAR(256), nullable=False, index=True) method = Column(String, nullable=False, index=True) - request_id = Column(BigInteger, nullable=False, index=True) - # TODO(zomglings): Should we conditional indices on parameters depending on the contract type? + request_id = Column(DECIMAL, nullable=False, index=True) parameters = Column(JSONB, nullable=False) expires_at = Column(DateTime(timezone=True), nullable=True, index=True) From a7ab7063b04bbd8b12eef241ae99b0b1a4955676 Mon Sep 17 00:00:00 2001 From: kompotkot Date: Thu, 10 Aug 2023 15:55:27 +0000 Subject: [PATCH 4/4] Clarified errors message for duplication of call request --- .../versions/040f2dfde5a5_request_id_decimal_column.py | 6 +++--- engineapi/engineapi/routes/metatx.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py b/engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py index f9315a8b..382f2074 100644 --- a/engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py +++ b/engineapi/alembic/versions/040f2dfde5a5_request_id_decimal_column.py @@ -62,9 +62,9 @@ DELETE FROM call_requests WHERE id IN (SELECT id FROM DeleteDuplicates);""") op.alter_column("call_requests", "request_id", nullable=False) # Other - # op.create_unique_constraint(op.f('uq_blockchains_id'), 'blockchains', ['id']) - # op.create_unique_constraint(op.f('uq_call_request_types_name'), 'call_request_types', ['name']) - # op.create_unique_constraint(op.f('uq_metatx_requesters_id'), 'metatx_requesters', ['id']) + op.create_unique_constraint(op.f('uq_blockchains_id'), 'blockchains', ['id']) + op.create_unique_constraint(op.f('uq_call_request_types_name'), 'call_request_types', ['name']) + op.create_unique_constraint(op.f('uq_metatx_requesters_id'), 'metatx_requesters', ['id']) # ### end Alembic commands ### diff --git a/engineapi/engineapi/routes/metatx.py b/engineapi/engineapi/routes/metatx.py index 6ba090b1..3af6bac9 100644 --- a/engineapi/engineapi/routes/metatx.py +++ b/engineapi/engineapi/routes/metatx.py @@ -385,7 +385,7 @@ async def create_requests( except contracts_actions.CallRequestAlreadyRegistered: raise EngineHTTPException( status_code=409, - detail="Call request with same parameters registered", + detail="Call request with same request_id already registered", ) except Exception as err: logger.error(repr(err))