Identity command improvements #11

jdetter · 2023-06-21T23:10:19Z

Description of Changes

Commands that take an identity can now typically either take an identity or the name of an identity. In certain places like creating identities or setting a name you may have to either specify an identity or a name and they are not interchangable.
I've also cleaned up some of my other TODOs that I had remaining

New Features

You can now delete all identities by using --all in spacetime identity remove:

boppy@boppy-macbook SpacetimeDB % spacetime identity new --no-email --name ident
 IDENTITY     a30ac9225d2fb8b22d0e24b36b72b1f7b5fc620716ef0c7d4354f58603f12232
 NAME         ident
 EMAIL        
boppy@boppy-macbook SpacetimeDB % spacetime identity new --no-email --name ident2
 IDENTITY     6d8502416f091c64f913323a179649bd0ead148c4ee2bb5ae3ad41553b3efb28
 NAME         ident2
 EMAIL        
boppy@boppy-macbook SpacetimeDB % spacetime identity remove --all
Are you sure you want to remove all identities? (y/n) y
 2 identities removed.
boppy@boppy-macbook SpacetimeDB %

spacetime identity token can now be used to print out the token for an identity. Either the name of the identity or the identity itself can be passed in.

boppy@boppy-macbook SpacetimeDB % spacetime identity token my_actual_identity
eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NiJ9.eyJoZXhfaWRlbnRpdHkiOiI1OGM3M2E2NTUyOTRjZWU0NzBhYmY2MjY1NjE2Yjk0YzNiZGNiMTE5Mzc5MjkyN2I2YTVhMjk3MmJmMWI2ZGZhIiwiaWF0IjoxNjg3Mzk0NDc3fQ.UOoJKH73E5HekTn1Ldc6WCkYhoKiPa-JwqwRQhRb-UMg7NwCzocMH1aYhR01fMfdWVNf9b9Es6ozV5JyWYKSVQ

You can also name or rename an identity using spacetime identity set-name:

boppy@boppy-macbook SpacetimeDB % spacetime identity new --no-email
 IDENTITY     58c73a655294cee470abf6265616b94c3bdcb1193792927b6a5a2972bf1b6dfa
 NAME         
 EMAIL        
boppy@boppy-macbook SpacetimeDB % spacetime identity set-name 58c73a655294cee470abf6265616b94c3bdcb1193792927b6a5a2972bf1b6dfa ident
 IDENTITY  58c73a655294cee470abf6265616b94c3bdcb1193792927b6a5a2972bf1b6dfa
 NAME      ident
boppy@boppy-macbook SpacetimeDB % spacetime identity set-name ident my_actual_identity
 IDENTITY  58c73a655294cee470abf6265616b94c3bdcb1193792927b6a5a2972bf1b6dfa
 NAME      my_actual_identity
boppy@boppy-macbook SpacetimeDB %

Cards

This completes the following notion cards:

https://www.notion.so/clockworklabs/CLI-Flag-for-printing-identity-token-2f71e3ba23af4b47b3a6e1ec3569b841?pvs=4

API

This is a breaking change to the module API
This is a breaking change to the ClientAPI

If the API is breaking, please state below what will break

gefjon

I've read and written "identity" so many times that it no longer looks like a word.

gefjon · 2023-06-22T14:12:47Z

crates/cli/src/config.rs

+        let configs = self.identity_configs_mut();
+        for config in configs.iter_mut() {
+            if config.identity == identity {
+                config.nickname = Some(name);


Should this warn if the identity already has a nickname?

Good suggestion. Added

gefjon · 2023-06-22T14:19:21Z

crates/cli/src/config.rs


+    pub fn set_identity_nickname(&mut self, identity: String, name: String) -> Result<(), anyhow::Error> {
+        let configs = self.identity_configs_mut();
+        for config in configs.iter_mut() {


You can probably rewrite your for loop here to use Iterator::find, which might look like:

let config = configs.iter_mut() .find(|config| config.identity == identity) // Returns Option<&mut WhateverTypeConfigsAre> .ok_or_else(|| anyhow::anyhow!("Identity {} not found", identity) // Converts Option to Result; closure to avoid formatting unless necessary ?; // Bails if not found config.nickname = Some(name); Ok(())

Whether you think that's more clear is up to you.

gefjon · 2023-06-22T14:22:11Z

crates/cli/src/config.rs

+                &self
+                    .identity_configs()
+                    .iter()
+                    .find(|c| c.identity == identity_or_name.clone())


I suspect this clone is unnecessary; you might be able to convince rustc by writing &c.identity == &identity_or_name. In my experience, getting the borrows right on find closures is always a bit of trial and error.

gefjon · 2023-06-22T14:24:30Z

crates/cli/src/config.rs

    }

+    /// Converts some name or identity to an identity. If the input is None, None is returned. If an
+    /// identity is looked up and it doesn't exist, we panic.


Would be nice to add a bit to this comment about how map_name_to_identity is only called in contexts where the identity_or_name is known to exist (or be None), so the panic should be unreachable. Assuming that's the case.

gefjon · 2023-06-22T14:26:03Z

crates/cli/src/config.rs


+    /// Converts some name or identity to an identity. If the input is None, None is returned. If an
+    /// identity is looked up and it doesn't exist, we panic.
+    pub fn map_name_to_identity(&self, identity_or_name: Option<&String>) -> Option<&String> {


Should probably take identity_or_name: Option<&str>.

map_ function name makes me assume this is a higher-order function like Iterator::map. Perhaps resolve_name_to_identity is better?

crates/cli/src/subcommands/identity.rs

gefjon · 2023-06-22T14:43:17Z

crates/cli/src/subcommands/identity.rs

 async fn exec_set_email(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
    let email = args.get_one::<String>("email").unwrap().clone();
-    let identity = args.get_one::<String>("identity").unwrap().clone();
+    let identity = config.map_name_to_identity(args.get_one::<String>("identity")).unwrap();


Should probably expect here to get a better error message.

gefjon · 2023-06-22T14:44:14Z

crates/cli/src/subcommands/identity.rs

 async fn exec_recover(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
    let email = args.get_one::<String>("email").unwrap();
-    let identity = args.get_one::<String>("identity").unwrap().to_lowercase();
+    let identity = config.map_name_to_identity(args.get_one::<String>("identity")).unwrap();


Should probably also be an expect.

gefjon · 2023-06-22T14:46:41Z

crates/cli/src/util.rs

-            Ok(identity_config.clone())
+    if let Some(identity_or_name) = identity_or_name {
+        if is_hex_identity(identity_or_name) {
+            if let Some(identity_config) = config.get_identity_config_by_identity(identity_or_name) {


You should be able to write this conditional more succinctly as something like:

config.get_identity_config_by_identity(identity_or_name) .map(Clone::clone) .ok_or_else(|| anyhow::anyhow!("Missing identity credentials for identity: {}", identity_or_name))

Whether you think that's better is up to you.

gefjon · 2023-06-22T14:48:00Z

crates/cli/src/util.rs

+            match config.map_name_to_identity(Some(&identity_or_name.to_string())) {
+                None => Err(anyhow::anyhow!("No such identity for name: {}", identity_or_name,)),
+                Some(_) => {
+                    if let Some(identity_config) = config.get_identity_config_by_name(identity_or_name) {


Conditional can be rewritten as Option combinators, like above.

Also, I believe this is the same conditional as above, so maybe it should be a named function.

Centril · 2023-06-22T15:07:45Z

crates/cli/src/config.rs

+        let configs = self.identity_configs_mut();
+        for config in configs.iter_mut() {
+            if config.identity == identity {
+                config.nickname = Some(name);
+                return Ok(());
+            }
+        }
+
+        Err(anyhow::anyhow!("Identity {} not found", identity))


Suggested change

let configs = self.identity_configs_mut();

for config in configs.iter_mut() {

if config.identity == identity {

config.nickname = Some(name);

return Ok(());

}

}

Err(anyhow::anyhow!("Identity {} not found", identity))

self.get_identity_config_by_identity_mut(&identity)

.ok_or_else(|| anyhow::anyhow!("Identity {} not found", identity))

.map(|c| c.nickname = Some(name))

Centril · 2023-06-22T15:14:38Z

crates/cli/src/config.rs

        self.home.default_identity = Some(default_identity);
    }

+    pub fn set_identity_nickname(&mut self, identity: String, name: String) -> Result<(), anyhow::Error> {


Let's add doc comments.

crates/cli/src/config.rs

crates/cli/src/subcommands/identity.rs

Centril · 2023-06-22T15:15:52Z

crates/cli/src/subcommands/identity.rs

+async fn exec_token(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
+    let identity_or_name = config
+        .map_name_to_identity(args.get_one::<String>("identity"))
+        .unwrap()
+        .clone();
+    let ic = config
+        .get_identity_config_by_identity(identity_or_name.as_str())
+        .unwrap();
+    println!("{}", ic.token);
+    Ok(())
+}
+
+async fn exec_set_name(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {
+    let cloned_config = config.clone();
+    let identity_or_name = cloned_config
+        .map_name_to_identity(args.get_one::<String>("identity"))
+        .unwrap();
+    let new_name = args.get_one::<String>("name").unwrap();
+    config.set_identity_nickname(identity_or_name.clone(), new_name.clone())?;
+    config.save();
+    let ic = config
+        .get_identity_config_by_identity(identity_or_name.as_str())
+        .unwrap();
+    print_identity_config(ic);
+    Ok(())
+}
+
 async fn exec_set_email(config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> {


Doc comments

crates/cli/src/util.rs

Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com> Signed-off-by: John Detter <4099508+jdetter@users.noreply.github.com>

cloutiertyler

LGTM

* Working on improving commands that use identities * Fix lints * Reverted file that shouldn't have changed * Found and fixed all other todos * Addressed more CLI TODOs * Fixes for formatting issues * Set names of identities * Set name of identities + clippy * Small fix * Added the start of a doc comment, switching over to another PR * Fixed tests that needed to be updated * Addressed more feedback and fixed several clippy issues * Small fix * Apply suggestions from code review Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com> Signed-off-by: John Detter <4099508+jdetter@users.noreply.github.com> * Added more doc comments * Addressing more feedback * Fixed really old bug in SpacetimeDB * Tests to verify new functionality * Fix clippy lints * Email during identity creation is optional * Fix output so testsuite passes --------- Signed-off-by: John Detter <4099508+jdetter@users.noreply.github.com> Co-authored-by: Boppy <no-reply@boppygames.gg> Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>

Co-authored-by: NateTheDev1 <nthnlrichards@gmail.com>

* Circle splitting and recombining; Client re-coloring * Renamed Entity.id into Entity.entity_id * Client refactor; Polish; Bugfixes * Store username between game runs * Renamed EntityActor.OnUpdate to OnEntityUpdate * [WIP] C# server * [WIP] C# server * Changed `DateTimeOffset` to `long` in C# module * Parallax background * Animate entities when they get consumed * Fixed respawn button * Draft into Steve's branch (#12) * Small changes * Changes to align with the tutorial. * Fixed rebase issue * Merged ConnectionManager and EntityManager into GameManager for tutorial simplicity * Renaming Actor -> Controller for simplicity's sake, although Actor is a good name * actor -> entityController * Merged ArenaController into GameManager for simplicity and cleaned up a few things * Small cleanup to keep it in line with the tutorial * Small changes to how overlapping works * Rebrand to Blackholio * Rebrand to Blackholio (missed one) * Added a README.md * fixed readme --------- Co-authored-by: Steve Gibson <steve@clockwokrlabs.io> Co-authored-by: Tyler Cloutier <cloutiertyler@users.noreply.github.com> Co-authored-by: Tyler Cloutier <cloutiertyler@aol.com>

Fixes a margin for the figure that kinda breaks it on mobile

Co-authored-by: John <no-reply@boppygames.gg>

Co-authored-by: NateTheDev1 <nthnlrichards@gmail.com>

Co-authored-by: John <no-reply@boppygames.gg>

jdetter force-pushed the jdetter/identity-unification branch from 1745129 to 663c45a Compare June 21, 2023 23:11

jdetter marked this pull request as ready for review June 21, 2023 23:57

jdetter changed the title ~~Identity and name are interchangable~~ Identity command improvements Jun 22, 2023

gefjon reviewed Jun 22, 2023

View reviewed changes

Centril reviewed Jun 22, 2023

View reviewed changes

jdetter and others added 20 commits June 22, 2023 22:26

Working on improving commands that use identities

08c656c

Fix lints

383c2b6

Reverted file that shouldn't have changed

1a8ce04

Found and fixed all other todos

af82aee

Addressed more CLI TODOs

a4cbe56

Fixes for formatting issues

fcd9256

Set names of identities

7650702

Set name of identities + clippy

8b47fa2

Small fix

50393c4

Added the start of a doc comment, switching over to another PR

4074390

Fixed tests that needed to be updated

48b72d3

Addressed more feedback and fixed several clippy issues

39db37c

Small fix

2044a74

Apply suggestions from code review

c03a510

Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com> Signed-off-by: John Detter <4099508+jdetter@users.noreply.github.com>

Added more doc comments

f42d031

Addressing more feedback

cdcc561

Fixed really old bug in SpacetimeDB

fe0cce3

Tests to verify new functionality

3bbb510

Fix clippy lints

be28a8f

Email during identity creation is optional

8cbe512

jdetter force-pushed the jdetter/identity-unification branch from 0bbb53d to 8cbe512 Compare June 23, 2023 03:27

Fix output so testsuite passes

9865358

jdetter requested a review from cloutiertyler June 23, 2023 04:42

cloutiertyler approved these changes Jun 23, 2023

View reviewed changes

cloutiertyler merged commit 1549440 into master Jun 23, 2023

cloutiertyler deleted the jdetter/identity-unification branch August 1, 2023 21:54

bfops pushed a commit that referenced this pull request Jul 16, 2025

Allow to use binary format (#11)

a6ec96e

Co-authored-by: NateTheDev1 <nthnlrichards@gmail.com>

bfops pushed a commit that referenced this pull request Jul 17, 2025

Merge pull request #11 from clockworklabs/cloutiertyler-patch-1

5129f3e

Fixes a margin for the figure that kinda breaks it on mobile

bfops pushed a commit that referenced this pull request Jul 17, 2025

Fix compilation issue (#11)

ac573a0

Co-authored-by: John <no-reply@boppygames.gg>

bfops pushed a commit that referenced this pull request Aug 7, 2025

Allow to use binary format (#11)

375bb48

Co-authored-by: NateTheDev1 <nthnlrichards@gmail.com>

bfops pushed a commit that referenced this pull request Aug 7, 2025

Fix compilation issue (#11)

37ace35

Co-authored-by: John <no-reply@boppygames.gg>

Identity command improvements #11

Identity command improvements #11

Uh oh!

Conversation

jdetter commented Jun 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Changes

New Features

Cards

API

Uh oh!

gefjon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cloutiertyler left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jdetter commented Jun 21, 2023 •

edited

Loading