Page MenuHomePhabricator

[services][identity] register_user RPC
ClosedPublic

Authored by varun on Jun 27 2022, 1:06 AM.
Tags
None
Referenced Files
F3392510: D4363.diff
Sat, Nov 30, 8:56 AM
Unknown Object (File)
Wed, Nov 27, 2:27 PM
Unknown Object (File)
Wed, Nov 27, 12:35 PM
Unknown Object (File)
Wed, Nov 13, 2:08 AM
Unknown Object (File)
Wed, Nov 13, 2:08 AM
Unknown Object (File)
Wed, Nov 13, 2:08 AM
Unknown Object (File)
Wed, Nov 13, 2:08 AM
Unknown Object (File)
Wed, Nov 13, 2:05 AM

Details

Summary

Implementation of the register_user RPC. We only need to do this for PAKE -- there is no registration process for wallet auth. The bulk of the logic is performed in the subroutines. This function handles updating some state and sending messages to the outbound stream.

Test Plan

called the RPC and tested all failure cases as well as the success case. subsequently tried logging in and received a new access token

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested changes to this revision.Jun 27 2022, 4:49 AM

This code is deeply nested which makes it unreadable. Please find a way to improve the readability.

services/identity/src/service.rs
121 ↗(On Diff #13821)

Is it a good idea to clone a client? What happens with connections when a DatabaseClient is cloned?

This revision now requires changes to proceed.Jun 27 2022, 4:49 AM

improve readability by decreasing indentation

services/identity/src/service.rs
121 ↗(On Diff #13821)

it's not a great idea, but it should be fine for v0.1. I think we'll want to have a client pool. I've created a linear task here: https://linear.app/comm/issue/ENG-1297/create-pool-of-database-clients

Looks ok! It would be great if you could reduce the indentation even more.

services/identity/src/service.rs
121 ↗(On Diff #13821)

Ok, makes sense to do it as a followup. But have you checked if it works at all, with a couple of concurrent clients?

This revision is now accepted and ready to land.Jun 28 2022, 6:43 AM
services/identity/src/service.rs
121 ↗(On Diff #13821)

Yeah, it works with like 5 or 6 concurrent clients.

In D4363#123685, @palys-swm wrote:

Looks ok! It would be great if you could reduce the indentation even more.

I'll refactor the if let stuff a little more before landing

This revision was automatically updated to reflect the committed changes.