#Discussion email verification to use listeners

1 messages · Page 1 of 1 (latest)

willow turretBOT
#
Notes for Discussion email verification to use listeners
At your assistance

@vague breach

No Response?

If no response in a reasonable time, ping @Member.

Closing

To close, type !solve or byte solve.

MCVE

Please include an MCVE so that we can reproduce your issue locally.

vague breach
#

so currently as you can see the initiate_email_verification is sending the email verification and creating or updating the user, so now that listeners came in my eye, i am not sure how do i refactor this, since from the code i saw so for emits are usually done from the controller layer, using request.app.emit ... but my email sending happens inside the service layer, so i am not sure how should i refactor this ... @naive marsh you have probably worked on this for litestar fullstack, so probably you would know best

naive marsh
#

you would probably refactor your service to somehow return an indicator on whether it should send and email or not. then you add the emitter back into the route. Without creating a custom emitter backend, you'll need to have access to the .emit function on the route to send the necessary signal.

But it would be nice to expand the backend to allow for an external signal at some point.

In your code, instead of calling this you'd return a boolean or something

vague breach
naive marsh
#

correct - and then grab a new db session if you need to look up extra data from what oyu passed in

#

do not pass the db session in the emit

vague breach
#

also i dont get why you refetch the account object in the listener, cant we just pass the account object instead of the account id?

naive marsh
#

no, not always in sqlalchemy - it has a complicated local cache state that can trigger strange things.

Also, I usually want to ensure that the work done there is in it's own transaction.

#

if it's like a pydantic model or something, that would be fine

vague breach
#

oh, is that the reason you said to grab a new session above

naive marsh
#

yep

#

yes, it does emit another set of DB queriers, but this is almost never a problem in this kind of situation and the tradeoff is no expiration issues, no object fetch issues, etc.

vague breach
#

for something like this

@post("/email-verification/request", status_code=status_codes.HTTP_204_NO_CONTENT)
    async def request_email_verification(
        self,
        data: RequestEmailVerificationRequest,
        account_service: AccountService,
        smtp_client: SMTPClient,
        captcha_client: CaptchaClient,
    ) -> None:
        """Request email verification for an account."""
        await account_service.request_email_verification(
            email=data.email,
            smtp_client=smtp_client,
            captcha_client=captcha_client,
            captcha_token=data.captcha,
        )

# in service
async def request_email_verification(
        self,
        email: str,
        smtp_client: SMTPClient,
        captcha_client: captcha.CaptchaClient,
        captcha_token: str | UnsetType,
    ) -> None:
        await self._verify_captcha(captcha_client, captcha_token)

        account = await self.get_one_or_none(email=email)

        # we dont want to report any errors to the client,
        # as it will lead to account enumeration.

        if account is None:
            return

        vs = account.email_verification_status

        if vs is EmailVerificationStatus.PENDING:
            await self.initiate_email_verification(account, smtp_client)
        elif vs is EmailVerificationStatus.VERIFIED:
            await self.initiate_password_reset(account, smtp_client)
        # MOVING status is ignored here because email change verification
        # was sent to the new email address, not this one. If the user needs
        # to resend it, they should do so from account settings.

i would have to tell the controller which event to emit too right since this can trigger two things, wont this get ugly?

naive marsh
#

exactly - and this is where using exists or something like that on the repo/service can help as well. you can do simple guards on these types of functions.

#

basically i try to make sure all of my jobs fetch their info and hold db sessions as short as possible

vague breach
naive marsh
#

because sqlalchemy can obfuscate how many queries it's emitting and when they emit, I tend to not pass them as arguments to functions.

you'll also get the added benefit of being ready for horizontal scaling when you need to move this work to mutliple processes/servers/or both. Each process can fetch the data it needs and you are passing primitives around instead.

naive marsh
# vague breach can you elaborate this a bit, didnt really get you

this looks like a guard function almost:

        await self._verify_captcha(captcha_client, captcha_token)

        account = await self.get_one_or_none(email=email)

        # we dont want to report any errors to the client,
        # as it will lead to account enumeration.

        if account is None:
            return

let's say you didn't need the rest of account in the email body. you could just do something like this:

  await self._verify_captcha(captcha_client, captcha_token)
  account_exists = await self.exits(email=email)
  if account_exists is None:
      return
vague breach
#

oh right exists will be faster

#

if i would write pure SQL, i would do a select exists check, but since i am doing alchemy and all i just forgot that

naive marsh
#

also, you may see me do more walrus operators, but that's just a preference - it's all the same.

if (account := (await self.get_one_or_none(email=email))) is None:
  return
vague breach
#

but in this case i do need the acount below to check the email verification status

vague breach
naive marsh
#

does it?

#

if you have details, let me know. I wasn't aware of any issues other than it being sometimes hard to understand

vague breach
vague breach
#

@naive marsh also one more thing, up until now since i used to send the email in the request response cycle only, so the tests i wrote which used to check if the email was sent after user creation and all, all break now so is there any pattern i should follow their to check the emails

#

the solution i can think of is to just wait for the messages to arrive in (mailpit (testing tool)) with a timeout

naive marsh
vague breach
#

but anyways whats the approach? i will have to wait for the messages to arrive right? since they arent now instantly sent in the request response cycle

naive marsh
#

yes, it should basically be instant though.

vague breach
#

yeah so i like keep a timeout of 1 sec?

naive marsh
naive marsh
vague breach
# 
async def wait_for_messages(
        self,
        email: str,
        count: int = 1,
        interval: float = 0.1,
    ) -> list[dict[str, Any]]:
        while True:
            messages = await self.get_messages_for(email)
            if len(messages) >= count:
                return messages
            await asyncio.sleep(interval)

so i have this method, and in the tests i do something like this

async with asyncio.timeout(2.0):
        messages = await mailpit.wait_for_messages("[email protected]")
naive marsh
#

if it's working, go for it

vague breach
#

both manual and automated

naive marsh
#

yes, mailpit is definitely just a dev tool

vague breach
#

i run the mailpit container using the testcontainers lib in the tests, and when its manual it is just docker compose up

#

is timeout=1 a good threshold like where a platform specific things wont matter?

#

i am just worried about that, since its passing in the local testing but maybe it will fail in ci/cd?

naive marsh
#

i'd say if it takes more than a second to send an email to a local docker container that's a problem. I don't know how long mailpit takes to register a received mail, that would be the only thing about it

#

how are you tesing that it sent now?

#

the timeout method?

vague breach
# naive marsh how are you tesing that it sent now? 
async def get_messages_for(self, email: str) -> list[dict[str, Any]]:
        messages = await self.get_messages()
        return [
            msg
            for msg in messages
            if any(to.get("Address") == email for to in msg.get("To", []))
        ]

yeah it just calls this method

#

i see that you have that await_events thing i can add that too, then it will be basically perfect right

#

but its basically the same asyncio.sleep i wrote i guess

stiff rootBOT
#

src/py/tests/utils/events.py line 8

async def wait_for_events(max_wait: float = 1.0, interval: float = 0.1) -> None:
vague breach
stiff rootBOT
#

src/py/tests/utils/events.py line 8

async def wait_for_events(max_wait: float = 1.0, interval: float = 0.1) -> None: