Page MenuHomePhabricator

[services][identity] register_user RPC
ClosedPublic

Authored by varun on Jun 27 2022, 1:06 AM.
Tags
None
Referenced Files
F3378705: D4363.id13857.diff
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
Unknown Object (File)
Thu, Oct 31, 10:30 PM
Unknown Object (File)
Thu, Oct 31, 10:22 PM

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
Branch
varun/identity_service (branched from master)
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

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

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

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

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.