Page MenuHomePhabricator

[services] Tunnelbroker - Add `updateSessionItemDeviceToken` database method
ClosedPublic

Authored by max on Sep 19 2022, 7:04 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 26, 11:40 PM
Unknown Object (File)
Sat, Oct 26, 11:46 AM
Unknown Object (File)
Sat, Oct 26, 2:46 AM
Unknown Object (File)
Tue, Oct 15, 4:42 PM
Unknown Object (File)
Oct 9 2024, 2:32 AM
Unknown Object (File)
Oct 9 2024, 2:32 AM
Unknown Object (File)
Oct 9 2024, 2:32 AM
Unknown Object (File)
Oct 9 2024, 2:31 AM

Details

Summary

This diff introduces a database method to update the device notification token field.
The purpose is to update the device notification token when the token is expired or wrong.

Related Linear task: ENG-1782

Test Plan
  1. Successfully built the server.
  2. Run the test in D5184 using yarn run-unit-tests and yarn run-integration-tests.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

max held this revision as a draft.
max published this revision for review.Sep 19 2022, 7:08 AM
max edited the summary of this revision. (Show Details)
max edited the test plan for this revision. (Show Details)
max added a reviewer: tomek.

not familiar with Aws sdk, but the C++ looks about right. I feel like having a test case for this would help a lot of asserting that it has the desired behavior

max removed a reviewer: karol.
In D5179#151896, @jon wrote:

I feel like having a test case for this would help a lot of asserting that it has the desired behavior

Sounds reasonable! D5184 was created and the test description was updated as well.

tomek requested changes to this revision.Sep 20 2022, 7:09 AM
tomek added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
141–145 ↗(On Diff #16837)

Is there a way to inform the caller that this operation failed? Assuming that it always succeeds sounds dangerous.

This revision now requires changes to proceed.Sep 20 2022, 7:09 AM

Changes to return bool result instead of void.

max added inline comments.
services/tunnelbroker/src/Database/DatabaseManager.cpp
141–145 ↗(On Diff #16837)

Is there a way to inform the caller that this operation failed? Assuming that it always succeeds sounds dangerous.

I think it's a good idea to get rid of voiding everywhere practice in the internal database functions. I've changed it to the bool, and we will return grpc::Status(grpc::StatusCode::INTERNAL, "Can not update the token in the Database"); to the client in the follow-up diff where it handles.

We can always revisit if we want more detail than just a bool to denote success/failure. But that decision can be deferred.

This looks fine to me.

This revision is now accepted and ready to land.Sep 23 2022, 4:49 AM
max marked an inline comment as done.

Rebasing.