Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

fix(account): update stored procedures to be more replication friendly #410

Merged
merged 1 commit into from Oct 18, 2018

Conversation

vbudhram
Copy link
Contributor

@vbudhram vbudhram commented Oct 18, 2018

This PR updates our stored procedures to be more replication friendly and is targeted against train-121.

  • verifyTokensWithMethod_2
  • verifyTokenCode_1

@vbudhram vbudhram added the WIP label Oct 18, 2018
@vbudhram vbudhram added this to the FxA-0: quality milestone Oct 18, 2018
@ghost ghost assigned vbudhram Oct 18, 2018
@ghost ghost added the waffle:active label Oct 18, 2018
END;

-- Update session verification methods
UPDATE `sessionTokens` SET verificationMethod = verificationMethodArg, verifiedAt = verifiedAtArg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I significantly tweaked this procedure to only update the verificationMethod on the session table. All previous logic was pulled into the db.verifyTokensWithMethod procedure.

WHERE tokenVerificationId = @tokenVerificationId
AND uid = uidArg;

DELETE FROM unverifiedTokens
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I simplified this by removing the transaction portion, which will return the affectedRows from the DELETE. I don't remember the original logic why I put this in a transaction but think it is ok to remove.

Copy link
Contributor

Choose a reason for hiding this comment

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

What a pain. It does kinda wanna be a transaction (I logged it, I did it), but...

@vbudhram
Copy link
Contributor Author

@mozilla/fxa-devs @jrgm @rfk Thoughts on approach?

@vbudhram vbudhram requested review from rfk, jrgm and a team October 18, 2018 18:32
@vbudhram vbudhram removed the WIP label Oct 18, 2018
Copy link
Contributor

@jrgm jrgm left a comment

Choose a reason for hiding this comment

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

lgtm

@vbudhram vbudhram merged commit 1c36f97 into train-121 Oct 18, 2018
@ghost ghost removed the waffle:review label Oct 18, 2018
@vbudhram vbudhram mentioned this pull request Oct 19, 2018
@vbudhram vbudhram deleted the train-121-fix-replication branch October 26, 2018 15:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants