identity: bring back the login to hold that info from bridges (purely informational)

Michael Muré created

Change summary

bridge/github/import.go         |  2 +
bridge/gitlab/import.go         |  1 
bridge/jira/import.go           |  3 +
bridge/launchpad/import.go      |  1 
cache/bug_excerpt.go            | 15 ++++++++++-
cache/filter.go                 |  3 +
cache/identity_excerpt.go       | 17 ++++++++++++-
cache/repo_cache.go             | 10 ++++----
commands/user.go                |  3 ++
commands/user_create.go         |  2 
graphql/graph/gen_graph.go      | 43 +++++++++++++++++++++++++++++++++++
graphql/models/lazy_identity.go | 13 ++++++++++
graphql/schema/identity.graphql |  2 +
identity/bare.go                | 37 +++++++++++++++++++++++++----
identity/bare_test.go           |  1 
identity/common.go              |  2 
identity/identity.go            | 22 ++++++++++++++++-
identity/interface.go           | 12 +++++++++
identity/version.go             | 18 ++++++++++++--
19 files changed, 183 insertions(+), 24 deletions(-)

Detailed changes

bridge/github/import.go 🔗

@@ -546,6 +546,7 @@ func (gi *githubImporter) ensurePerson(repo *cache.RepoCache, actor *actor) (*ca
 	i, err = repo.NewIdentityRaw(
 		name,
 		email,
+		string(actor.Login),
 		string(actor.AvatarUrl),
 		map[string]string{
 			metaKeyGithubLogin: string(actor.Login),
@@ -592,6 +593,7 @@ func (gi *githubImporter) getGhost(repo *cache.RepoCache) (*cache.IdentityCache,
 	return repo.NewIdentityRaw(
 		name,
 		"",
+		string(q.User.Login),
 		string(q.User.AvatarUrl),
 		map[string]string{
 			metaKeyGithubLogin: string(q.User.Login),

bridge/gitlab/import.go 🔗

@@ -394,6 +394,7 @@ func (gi *gitlabImporter) ensurePerson(repo *cache.RepoCache, id int) (*cache.Id
 	i, err = repo.NewIdentityRaw(
 		user.Name,
 		user.PublicEmail,
+		user.Username,
 		user.AvatarURL,
 		map[string]string{
 			// because Gitlab

bridge/jira/import.go 🔗

@@ -194,9 +194,10 @@ func (ji *jiraImporter) ensurePerson(repo *cache.RepoCache, user User) (*cache.I
 	i, err = repo.NewIdentityRaw(
 		user.DisplayName,
 		user.EmailAddress,
+		user.Key,
 		"",
 		map[string]string{
-			metaKeyJiraUser: string(user.Key),
+			metaKeyJiraUser: user.Key,
 		},
 	)
 

bridge/launchpad/import.go 🔗

@@ -33,6 +33,7 @@ func (li *launchpadImporter) ensurePerson(repo *cache.RepoCache, owner LPPerson)
 	return repo.NewIdentityRaw(
 		owner.Name,
 		"",
+		owner.Login,
 		"",
 		map[string]string{
 			metaKeyLaunchpadLogin: owner.Login,

cache/bug_excerpt.go 🔗

@@ -2,6 +2,7 @@ package cache
 
 import (
 	"encoding/gob"
+	"fmt"
 
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/entity"
@@ -42,11 +43,21 @@ type BugExcerpt struct {
 
 // identity.Bare data are directly embedded in the bug excerpt
 type LegacyAuthorExcerpt struct {
-	Name string
+	Name  string
+	Login string
 }
 
 func (l LegacyAuthorExcerpt) DisplayName() string {
-	return l.Name
+	switch {
+	case l.Name == "" && l.Login != "":
+		return l.Login
+	case l.Name != "" && l.Login == "":
+		return l.Name
+	case l.Name != "" && l.Login != "":
+		return fmt.Sprintf("%s (%s)", l.Name, l.Login)
+	}
+
+	panic("invalid person data")
 }
 
 func NewBugExcerpt(b bug.Interface, snap *bug.Snapshot) *BugExcerpt {

cache/filter.go 🔗

@@ -44,7 +44,8 @@ func AuthorFilter(query string) Filter {
 		}
 
 		// Legacy identity support
-		return strings.Contains(strings.ToLower(excerpt.LegacyAuthor.Name), query)
+		return strings.Contains(strings.ToLower(excerpt.LegacyAuthor.Name), query) ||
+			strings.Contains(strings.ToLower(excerpt.LegacyAuthor.Login), query)
 	}
 }
 

cache/identity_excerpt.go 🔗

@@ -2,6 +2,7 @@ package cache
 
 import (
 	"encoding/gob"
+	"fmt"
 	"strings"
 
 	"github.com/MichaelMure/git-bug/entity"
@@ -20,6 +21,7 @@ type IdentityExcerpt struct {
 	Id entity.Id
 
 	Name              string
+	Login             string
 	ImmutableMetadata map[string]string
 }
 
@@ -27,6 +29,7 @@ func NewIdentityExcerpt(i *identity.Identity) *IdentityExcerpt {
 	return &IdentityExcerpt{
 		Id:                i.Id(),
 		Name:              i.Name(),
+		Login:             i.Login(),
 		ImmutableMetadata: i.ImmutableMetadata(),
 	}
 }
@@ -34,13 +37,23 @@ func NewIdentityExcerpt(i *identity.Identity) *IdentityExcerpt {
 // DisplayName return a non-empty string to display, representing the
 // identity, based on the non-empty values.
 func (i *IdentityExcerpt) DisplayName() string {
-	return i.Name
+	switch {
+	case i.Name == "" && i.Login != "":
+		return i.Login
+	case i.Name != "" && i.Login == "":
+		return i.Name
+	case i.Name != "" && i.Login != "":
+		return fmt.Sprintf("%s (%s)", i.Name, i.Login)
+	}
+
+	panic("invalid person data")
 }
 
 // Match matches a query with the identity name, login and ID prefixes
 func (i *IdentityExcerpt) Match(query string) bool {
 	return i.Id.HasPrefix(query) ||
-		strings.Contains(strings.ToLower(i.Name), query)
+		strings.Contains(strings.ToLower(i.Name), query) ||
+		strings.Contains(strings.ToLower(i.Login), query)
 }
 
 /*

cache/repo_cache.go 🔗

@@ -1037,17 +1037,17 @@ func (c *RepoCache) NewIdentityFromGitUserRaw(metadata map[string]string) (*Iden
 // NewIdentity create a new identity
 // The new identity is written in the repository (commit)
 func (c *RepoCache) NewIdentity(name string, email string) (*IdentityCache, error) {
-	return c.NewIdentityRaw(name, email, "", nil)
+	return c.NewIdentityRaw(name, email, "", "", nil)
 }
 
 // NewIdentityFull create a new identity
 // The new identity is written in the repository (commit)
-func (c *RepoCache) NewIdentityFull(name string, email string, avatarUrl string) (*IdentityCache, error) {
-	return c.NewIdentityRaw(name, email, avatarUrl, nil)
+func (c *RepoCache) NewIdentityFull(name string, email string, login string, avatarUrl string) (*IdentityCache, error) {
+	return c.NewIdentityRaw(name, email, login, avatarUrl, nil)
 }
 
-func (c *RepoCache) NewIdentityRaw(name string, email string, avatarUrl string, metadata map[string]string) (*IdentityCache, error) {
-	i := identity.NewIdentityFull(name, email, avatarUrl)
+func (c *RepoCache) NewIdentityRaw(name string, email string, login string, avatarUrl string, metadata map[string]string) (*IdentityCache, error) {
+	i := identity.NewIdentityFull(name, email, login, avatarUrl)
 	return c.finishIdentity(i, metadata)
 }
 

commands/user.go 🔗

@@ -41,6 +41,8 @@ func runUser(cmd *cobra.Command, args []string) error {
 		switch userFieldsQuery {
 		case "email":
 			fmt.Printf("%s\n", id.Email())
+		case "login":
+			fmt.Printf("%s\n", id.Login())
 		case "humanId":
 			fmt.Printf("%s\n", id.Id().Human())
 		case "id":
@@ -67,6 +69,7 @@ func runUser(cmd *cobra.Command, args []string) error {
 	fmt.Printf("Id: %s\n", id.Id())
 	fmt.Printf("Name: %s\n", id.Name())
 	fmt.Printf("Email: %s\n", id.Email())
+	fmt.Printf("Login: %s\n", id.Login())
 	fmt.Printf("Last modification: %s (lamport %d)\n",
 		id.LastModification().Time().Format("Mon Jan 2 15:04:05 2006 +0200"),
 		id.LastModificationLamport())

commands/user_create.go 🔗

@@ -43,7 +43,7 @@ func runUserCreate(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	id, err := backend.NewIdentityRaw(name, email, avatarURL, nil)
+	id, err := backend.NewIdentityRaw(name, email, "", avatarURL, nil)
 	if err != nil {
 		return err
 	}

graphql/graph/gen_graph.go 🔗

@@ -200,6 +200,7 @@ type ComplexityRoot struct {
 		HumanID     func(childComplexity int) int
 		ID          func(childComplexity int) int
 		IsProtected func(childComplexity int) int
+		Login       func(childComplexity int) int
 		Name        func(childComplexity int) int
 	}
 
@@ -1100,6 +1101,13 @@ func (e *executableSchema) Complexity(typeName, field string, childComplexity in
 
 		return e.complexity.Identity.IsProtected(childComplexity), true
 
+	case "Identity.login":
+		if e.complexity.Identity.Login == nil {
+			break
+		}
+
+		return e.complexity.Identity.Login(childComplexity), true
+
 	case "Identity.name":
 		if e.complexity.Identity.Name == nil {
 			break
@@ -1941,6 +1949,8 @@ type Identity {
     name: String
     """The email of the person, if known."""
     email: String
+    """The login of the person, if known."""
+    login: String
     """A non-empty string to display, representing the identity, based on the non-empty values."""
     displayName: String!
     """An url to an avatar"""
@@ -5711,6 +5721,37 @@ func (ec *executionContext) _Identity_email(ctx context.Context, field graphql.C
 	return ec.marshalOString2string(ctx, field.Selections, res)
 }
 
+func (ec *executionContext) _Identity_login(ctx context.Context, field graphql.CollectedField, obj models.IdentityWrapper) (ret graphql.Marshaler) {
+	defer func() {
+		if r := recover(); r != nil {
+			ec.Error(ctx, ec.Recover(ctx, r))
+			ret = graphql.Null
+		}
+	}()
+	fc := &graphql.FieldContext{
+		Object:   "Identity",
+		Field:    field,
+		Args:     nil,
+		IsMethod: true,
+	}
+
+	ctx = graphql.WithFieldContext(ctx, fc)
+	resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (interface{}, error) {
+		ctx = rctx // use context from middleware stack in children
+		return obj.Login()
+	})
+	if err != nil {
+		ec.Error(ctx, err)
+		return graphql.Null
+	}
+	if resTmp == nil {
+		return graphql.Null
+	}
+	res := resTmp.(string)
+	fc.Result = res
+	return ec.marshalOString2string(ctx, field.Selections, res)
+}
+
 func (ec *executionContext) _Identity_displayName(ctx context.Context, field graphql.CollectedField, obj models.IdentityWrapper) (ret graphql.Marshaler) {
 	defer func() {
 		if r := recover(); r != nil {
@@ -11256,6 +11297,8 @@ func (ec *executionContext) _Identity(ctx context.Context, sel ast.SelectionSet,
 			out.Values[i] = ec._Identity_name(ctx, field, obj)
 		case "email":
 			out.Values[i] = ec._Identity_email(ctx, field, obj)
+		case "login":
+			out.Values[i] = ec._Identity_login(ctx, field, obj)
 		case "displayName":
 			out.Values[i] = ec._Identity_displayName(ctx, field, obj)
 			if out.Values[i] == graphql.Null {

graphql/models/lazy_identity.go 🔗

@@ -18,6 +18,7 @@ type IdentityWrapper interface {
 	Id() entity.Id
 	Name() string
 	Email() (string, error)
+	Login() (string, error)
 	AvatarUrl() (string, error)
 	Keys() ([]*identity.Key, error)
 	ValidKeysAtTime(time lamport.Time) ([]*identity.Key, error)
@@ -76,6 +77,14 @@ func (li *lazyIdentity) Email() (string, error) {
 	return id.Email(), nil
 }
 
+func (li *lazyIdentity) Login() (string, error) {
+	id, err := li.load()
+	if err != nil {
+		return "", err
+	}
+	return id.Login(), nil
+}
+
 func (li *lazyIdentity) AvatarUrl() (string, error) {
 	id, err := li.load()
 	if err != nil {
@@ -142,6 +151,10 @@ func (l loadedIdentity) Email() (string, error) {
 	return l.Interface.Email(), nil
 }
 
+func (l loadedIdentity) Login() (string, error) {
+	return l.Interface.Login(), nil
+}
+
 func (l loadedIdentity) AvatarUrl() (string, error) {
 	return l.Interface.AvatarUrl(), nil
 }

graphql/schema/identity.graphql 🔗

@@ -8,6 +8,8 @@ type Identity {
     name: String
     """The email of the person, if known."""
     email: String
+    """The login of the person, if known."""
+    login: String
     """A non-empty string to display, representing the identity, based on the non-empty values."""
     displayName: String!
     """An url to an avatar"""

identity/bare.go 🔗

@@ -27,6 +27,7 @@ type Bare struct {
 	id        entity.Id
 	name      string
 	email     string
+	login     string
 	avatarUrl string
 }
 
@@ -34,8 +35,8 @@ func NewBare(name string, email string) *Bare {
 	return &Bare{id: entity.UnsetId, name: name, email: email}
 }
 
-func NewBareFull(name string, email string, avatarUrl string) *Bare {
-	return &Bare{id: entity.UnsetId, name: name, email: email, avatarUrl: avatarUrl}
+func NewBareFull(name string, email string, login string, avatarUrl string) *Bare {
+	return &Bare{id: entity.UnsetId, name: name, email: email, login: login, avatarUrl: avatarUrl}
 }
 
 func deriveId(data []byte) entity.Id {
@@ -46,7 +47,7 @@ func deriveId(data []byte) entity.Id {
 type bareIdentityJSON struct {
 	Name      string `json:"name,omitempty"`
 	Email     string `json:"email,omitempty"`
-	Login     string `json:"login,omitempty"` // Deprecated, only kept to have the same ID when reading an old value
+	Login     string `json:"login,omitempty"`
 	AvatarUrl string `json:"avatar_url,omitempty"`
 }
 
@@ -54,6 +55,7 @@ func (i *Bare) MarshalJSON() ([]byte, error) {
 	return json.Marshal(bareIdentityJSON{
 		Name:      i.name,
 		Email:     i.email,
+		Login:     i.login,
 		AvatarUrl: i.avatarUrl,
 	})
 }
@@ -70,6 +72,7 @@ func (i *Bare) UnmarshalJSON(data []byte) error {
 
 	i.name = aux.Name
 	i.email = aux.Email
+	i.login = aux.Login
 	i.avatarUrl = aux.AvatarUrl
 
 	return nil
@@ -108,6 +111,11 @@ func (i *Bare) Email() string {
 	return i.email
 }
 
+// Login return the last version of the login
+func (i *Bare) Login() string {
+	return i.login
+}
+
 // AvatarUrl return the last version of the Avatar URL
 func (i *Bare) AvatarUrl() string {
 	return i.avatarUrl
@@ -126,13 +134,22 @@ func (i *Bare) ValidKeysAtTime(_ lamport.Time) []*Key {
 // DisplayName return a non-empty string to display, representing the
 // identity, based on the non-empty values.
 func (i *Bare) DisplayName() string {
-	return i.name
+	switch {
+	case i.name == "" && i.login != "":
+		return i.login
+	case i.name != "" && i.login == "":
+		return i.name
+	case i.name != "" && i.login != "":
+		return fmt.Sprintf("%s (%s)", i.name, i.login)
+	}
+
+	panic("invalid person data")
 }
 
 // Validate check if the Identity data is valid
 func (i *Bare) Validate() error {
-	if text.Empty(i.name) {
-		return fmt.Errorf("name is not set")
+	if text.Empty(i.name) && text.Empty(i.login) {
+		return fmt.Errorf("either name or login should be set")
 	}
 
 	if strings.Contains(i.name, "\n") {
@@ -143,6 +160,14 @@ func (i *Bare) Validate() error {
 		return fmt.Errorf("name is not fully printable")
 	}
 
+	if strings.Contains(i.login, "\n") {
+		return fmt.Errorf("login should be a single line")
+	}
+
+	if !text.Safe(i.login) {
+		return fmt.Errorf("login is not fully printable")
+	}
+
 	if strings.Contains(i.email, "\n") {
 		return fmt.Errorf("email should be a single line")
 	}

identity/bare_test.go 🔗

@@ -18,6 +18,7 @@ func TestBare_Id(t *testing.T) {
 
 func TestBareSerialize(t *testing.T) {
 	before := &Bare{
+		login:     "login",
 		email:     "email",
 		name:      "name",
 		avatarUrl: "avatar",

identity/common.go 🔗

@@ -37,7 +37,7 @@ func UnmarshalJSON(raw json.RawMessage) (Interface, error) {
 	b := &Bare{}
 
 	err = json.Unmarshal(raw, b)
-	if err == nil && b.name != "" {
+	if err == nil && (b.name != "" || b.login != "") {
 		return b, nil
 	}
 

identity/identity.go 🔗

@@ -56,13 +56,14 @@ func NewIdentity(name string, email string) *Identity {
 	}
 }
 
-func NewIdentityFull(name string, email string, avatarUrl string) *Identity {
+func NewIdentityFull(name string, email string, login string, avatarUrl string) *Identity {
 	return &Identity{
 		id: entity.UnsetId,
 		versions: []*Version{
 			{
 				name:      name,
 				email:     email,
+				login:     login,
 				avatarURL: avatarUrl,
 				nonce:     makeNonce(20),
 			},
@@ -282,6 +283,7 @@ func IsUserIdentitySet(repo repository.Repo) (bool, error) {
 
 type Mutator struct {
 	Name      string
+	Login     string
 	Email     string
 	AvatarUrl string
 	Keys      []*Key
@@ -292,6 +294,7 @@ func (i *Identity) Mutate(f func(orig Mutator) Mutator) {
 	orig := Mutator{
 		Name:      i.Name(),
 		Email:     i.Email(),
+		Login:     i.Login(),
 		AvatarUrl: i.AvatarUrl(),
 		Keys:      i.Keys(),
 	}
@@ -302,6 +305,7 @@ func (i *Identity) Mutate(f func(orig Mutator) Mutator) {
 	i.versions = append(i.versions, &Version{
 		name:      mutated.Name,
 		email:     mutated.Email,
+		login:     mutated.Login,
 		avatarURL: mutated.AvatarUrl,
 		keys:      mutated.Keys,
 	})
@@ -510,6 +514,11 @@ func (i *Identity) Email() string {
 	return i.lastVersion().email
 }
 
+// Login return the last version of the login
+func (i *Identity) Login() string {
+	return i.lastVersion().login
+}
+
 // AvatarUrl return the last version of the Avatar URL
 func (i *Identity) AvatarUrl() string {
 	return i.lastVersion().avatarURL
@@ -538,7 +547,16 @@ func (i *Identity) ValidKeysAtTime(time lamport.Time) []*Key {
 // DisplayName return a non-empty string to display, representing the
 // identity, based on the non-empty values.
 func (i *Identity) DisplayName() string {
-	return i.Name()
+	switch {
+	case i.Name() == "" && i.Login() != "":
+		return i.Login()
+	case i.Name() != "" && i.Login() == "":
+		return i.Name()
+	case i.Name() != "" && i.Login() != "":
+		return fmt.Sprintf("%s (%s)", i.Name(), i.Login())
+	}
+
+	panic("invalid person data")
 }
 
 // IsProtected return true if the chain of git commits started to be signed.

identity/interface.go 🔗

@@ -12,18 +12,30 @@ type Interface interface {
 	Id() entity.Id
 
 	// Name return the last version of the name
+	// Can be empty.
 	Name() string
 
 	// Email return the last version of the email
+	// Can be empty.
 	Email() string
 
+	// Login return the last version of the login
+	// Can be empty.
+	// Warning: this login can be defined when importing from a bridge but should *not* be
+	// used to identify an identity as multiple bridge with different login can map to the same
+	// identity. Use the metadata system for that usage instead.
+	Login() string
+
 	// AvatarUrl return the last version of the Avatar URL
+	// Can be empty.
 	AvatarUrl() string
 
 	// Keys return the last version of the valid keys
+	// Can be empty.
 	Keys() []*Key
 
 	// ValidKeysAtTime return the set of keys valid at a given lamport time
+	// Can be empty.
 	ValidKeysAtTime(time lamport.Time) []*Key
 
 	// DisplayName return a non-empty string to display, representing the

identity/version.go 🔗

@@ -24,7 +24,8 @@ type Version struct {
 	unixTime int64
 
 	name      string
-	email     string // as defined in git, not for bridges
+	email     string // as defined in git or from a bridge when importing the identity
+	login     string // from a bridge when importing the identity
 	avatarURL string
 
 	// The set of keys valid at that time, from this version onward, until they get removed
@@ -52,6 +53,7 @@ type VersionJSON struct {
 	UnixTime  int64             `json:"unix_time"`
 	Name      string            `json:"name,omitempty"`
 	Email     string            `json:"email,omitempty"`
+	Login     string            `json:"login,omitempty"`
 	AvatarUrl string            `json:"avatar_url,omitempty"`
 	Keys      []*Key            `json:"pub_keys,omitempty"`
 	Nonce     []byte            `json:"nonce,omitempty"`
@@ -81,6 +83,7 @@ func (v *Version) MarshalJSON() ([]byte, error) {
 		UnixTime:      v.unixTime,
 		Name:          v.name,
 		Email:         v.email,
+		Login:         v.login,
 		AvatarUrl:     v.avatarURL,
 		Keys:          v.keys,
 		Nonce:         v.nonce,
@@ -103,6 +106,7 @@ func (v *Version) UnmarshalJSON(data []byte) error {
 	v.unixTime = aux.UnixTime
 	v.name = aux.Name
 	v.email = aux.Email
+	v.login = aux.Login
 	v.avatarURL = aux.AvatarUrl
 	v.keys = aux.Keys
 	v.nonce = aux.Nonce
@@ -120,8 +124,8 @@ func (v *Version) Validate() error {
 		return fmt.Errorf("lamport time not set")
 	}
 
-	if text.Empty(v.name) {
-		return fmt.Errorf("name not set")
+	if text.Empty(v.name) && text.Empty(v.login) {
+		return fmt.Errorf("either name or login should be set")
 	}
 
 	if strings.Contains(v.name, "\n") {
@@ -132,6 +136,14 @@ func (v *Version) Validate() error {
 		return fmt.Errorf("name is not fully printable")
 	}
 
+	if strings.Contains(v.login, "\n") {
+		return fmt.Errorf("login should be a single line")
+	}
+
+	if !text.Safe(v.login) {
+		return fmt.Errorf("login is not fully printable")
+	}
+
 	if strings.Contains(v.email, "\n") {
 		return fmt.Errorf("email should be a single line")
 	}