From 06c21f00eb7c18fa421d4bec34c1c35b125f3af1 Mon Sep 17 00:00:00 2001 From: Blake Mizerany Date: Sun, 7 Apr 2024 18:38:48 -0700 Subject: [PATCH] x/model: make Digest just hold a string This is so much easier to reason about and allows Name to contain only parts, instead of a special field for the digest. It also removes the allocs in Digest.String(). --- x/model/digest.go | 20 ++++++++------------ x/model/digest_test.go | 6 +++--- x/model/name.go | 32 +++++++++++--------------------- x/model/name_test.go | 12 ++++++------ 4 files changed, 28 insertions(+), 42 deletions(-) diff --git a/x/model/digest.go b/x/model/digest.go index b6b08948..e093a575 100644 --- a/x/model/digest.go +++ b/x/model/digest.go @@ -15,21 +15,17 @@ import ( // // It is comparable with other Digests and can be used as a map key. type Digest struct { - typ string - digest string + s string } -func (d Digest) Type() string { return d.typ } -func (d Digest) Digest() string { return d.digest } -func (d Digest) IsValid() bool { return d != Digest{} } - -func (d Digest) String() string { - if !d.IsValid() { - return "" - } - return fmt.Sprintf("%s-%s", d.typ, d.digest) +func (d Digest) Type() string { + typ, _, _ := strings.Cut(d.s, "-") + return typ } +func (d Digest) IsValid() bool { return d.s != "" } +func (d Digest) String() string { return d.s } + func (d Digest) MarshalText() ([]byte, error) { return []byte(d.String()), nil } @@ -75,7 +71,7 @@ func (d Digest) Value() (driver.Value, error) { func ParseDigest(s string) Digest { typ, digest, ok := strings.Cut(s, "-") if ok && isValidDigestType(typ) && isValidHex(digest) { - return Digest{typ: typ, digest: digest} + return Digest{s: s} } return Digest{} } diff --git a/x/model/digest_test.go b/x/model/digest_test.go index 811b00f9..38bd8da3 100644 --- a/x/model/digest_test.go +++ b/x/model/digest_test.go @@ -16,9 +16,9 @@ import "testing" var testDigests = map[string]Digest{ "": {}, - "sha256-1234": {typ: "sha256", digest: "1234"}, - "sha256-5678": {typ: "sha256", digest: "5678"}, - "blake2-9abc": {typ: "blake2", digest: "9abc"}, + "sha256-1234": {s: "sha256-1234"}, + "sha256-5678": {s: "sha256-5678"}, + "blake2-9abc": {s: "blake2-9abc"}, "-1234": {}, "sha256-": {}, "sha256-1234-5678": {}, diff --git a/x/model/name.go b/x/model/name.go index 5b6df1f4..91e26d09 100644 --- a/x/model/name.go +++ b/x/model/name.go @@ -91,9 +91,8 @@ func (k PartKind) String() string { // // To make a Name by filling in missing parts from another Name, use [Fill]. type Name struct { - _ structs.Incomparable - parts [5]string // host, namespace, model, tag, build - digest Digest // digest is a special part + _ structs.Incomparable + parts [6]string // host, namespace, model, tag, build // TODO(bmizerany): track offsets and hold s (raw string) here? We // could pack the offests all into a single uint64 since the first @@ -140,12 +139,8 @@ func ParseName(s string) Name { if kind == PartInvalid { return Name{} } - if kind == PartDigest { - r.digest = ParseDigest(part) - if !r.digest.IsValid() { - return Name{} - } - continue + if kind == PartDigest && !ParseDigest(part).IsValid() { + return Name{} } r.parts[kind] = part } @@ -173,7 +168,7 @@ func (r Name) WithBuild(build string) Name { } func (r Name) WithDigest(digest Digest) Name { - r.digest = digest + r.parts[PartDigest] = digest.String() return r } @@ -244,7 +239,8 @@ var seps = [...]string{ PartNamespace: "/", PartModel: ":", PartTag: "+", - PartBuild: "", + PartBuild: "@", + PartDigest: "", } // WriteTo implements io.WriterTo. It writes the fullest possible display @@ -263,16 +259,12 @@ func (r Name) writeTo(w io.StringWriter) { if r.parts[i] == "" { continue } - if partsWritten > 0 { + if partsWritten > 0 || i == int(PartDigest) { w.WriteString(seps[i-1]) } w.WriteString(r.parts[i]) partsWritten++ } - if r.IsResolved() { - w.WriteString("@") - w.WriteString(r.digest.String()) - } } var builderPool = sync.Pool{ @@ -306,9 +298,6 @@ func (r Name) GoString() string { for i := range r.parts { r.parts[i] = cmp.Or(r.parts[i], "?") } - if !r.IsResolved() { - r.digest = Digest{"?", "?"} - } return r.String() } @@ -399,7 +388,7 @@ func (r Name) IsCompleteNoBuild() bool { // It is possible to have a valid Name, or a complete Name that is not // resolved. func (r Name) IsResolved() bool { - return r.digest.IsValid() + return r.Digest().IsValid() } // Digest returns the digest part of the Name, if any. @@ -407,7 +396,8 @@ func (r Name) IsResolved() bool { // If Digest returns a non-empty string, then [Name.IsResolved] will return // true, and digest is considered valid. func (r Name) Digest() Digest { - return r.digest + // This was already validated by ParseName, so we can just return it. + return Digest{r.parts[PartDigest]} } // EqualFold reports whether r and o are equivalent model names, ignoring diff --git a/x/model/name_test.go b/x/model/name_test.go index 1de3714d..a6c96b18 100644 --- a/x/model/name_test.go +++ b/x/model/name_test.go @@ -23,7 +23,7 @@ func fieldsFromName(p Name) fields { model: p.parts[PartModel], tag: p.parts[PartTag], build: p.parts[PartBuild], - digest: p.digest.String(), + digest: p.parts[PartDigest], } } @@ -103,7 +103,7 @@ var testNames = map[string]fields{ func TestNameParts(t *testing.T) { var p Name - if w, g := int(PartBuild+1), len(p.Parts()); w != g { + if w, g := int(PartDigest+1), len(p.Parts()); w != g { t.Errorf("Parts() = %d; want %d", g, w) } } @@ -235,7 +235,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "library/mistral:latest", wantComplete: "example.com/library/mistral:latest", wantModel: "mistral", - wantGoString: "example.com/library/mistral:latest+Q4_0@?-?", + wantGoString: "example.com/library/mistral:latest+Q4_0@?", }, { name: "Short Name", @@ -244,7 +244,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "mistral:latest", wantComplete: "mistral:latest", wantModel: "mistral", - wantGoString: "?/?/mistral:latest+?@?-?", + wantGoString: "?/?/mistral:latest+?@?", }, { name: "Long Name", @@ -253,7 +253,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "library/mistral:latest", wantComplete: "library/mistral:latest", wantModel: "mistral", - wantGoString: "?/library/mistral:latest+?@?-?", + wantGoString: "?/library/mistral:latest+?@?", }, { name: "Case Preserved", @@ -262,7 +262,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "Library/Mistral:Latest", wantComplete: "Library/Mistral:Latest", wantModel: "Mistral", - wantGoString: "?/Library/Mistral:Latest+?@?-?", + wantGoString: "?/Library/Mistral:Latest+?@?", }, { name: "With digest",