Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid copying Repository mutex state #364

Merged
merged 3 commits into from
Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions internal/slices/slice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
Copyright The ORAS Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package slices

// Clone returns a shallow copy of the slice.
func Clone[S ~[]E, E any](s S) S {
if s == nil {
return nil
}
return append(make(S, 0, len(s)), s...)
}
8 changes: 1 addition & 7 deletions registry/remote/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,5 @@ func (r *Registry) Repository(ctx context.Context, name string) (registry.Reposi
Registry: r.Reference.Registry,
Repository: name,
}
if err := ref.ValidateRepository(); err != nil {
return nil, err
}

repo := Repository(r.RepositoryOptions)
repo.Reference = ref
return &repo, nil
return newRepositoryWithOptions(ref, &r.RepositoryOptions)
abursavich marked this conversation as resolved.
Show resolved Hide resolved
}
5 changes: 2 additions & 3 deletions registry/remote/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,12 @@ func TestRegistry_Repository(t *testing.T) {
reg.MaxMetadataBytes = 8 * 1024 * 1024

ctx := context.Background()
want := &Repository{}
*want = Repository(reg.RepositoryOptions)
want.Reference.Repository = "hello-world"
got, err := reg.Repository(ctx, "hello-world")
if err != nil {
t.Fatalf("Registry.Repository() error = %v", err)
}
reg.Reference.Repository = "hello-world"
want := (*Repository)(&reg.RepositoryOptions)
if !reflect.DeepEqual(got, want) {
t.Errorf("Registry.Repository() = %v, want %v", got, want)
}
Expand Down
25 changes: 25 additions & 0 deletions registry/remote/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"oras.land/oras-go/v2/internal/httputil"
"oras.land/oras-go/v2/internal/ioutil"
"oras.land/oras-go/v2/internal/registryutil"
"oras.land/oras-go/v2/internal/slices"
"oras.land/oras-go/v2/internal/syncutil"
"oras.land/oras-go/v2/registry"
"oras.land/oras-go/v2/registry/remote/auth"
Expand Down Expand Up @@ -100,6 +101,8 @@ type Repository struct {
// If less than or equal to zero, a default (currently 4MiB) is used.
MaxMetadataBytes int64

// NOTE: Must keep fields in sync with newRepositoryWithOptions function.

// referrersState represents that if the repository supports Referrers API.
// default: referrersStateUnknown
referrersState referrersState
Expand All @@ -126,6 +129,28 @@ func NewRepository(reference string) (*Repository, error) {
}, nil
}

// newRepositoryWithOptions returns a Repository with the given Reference and
// RepositoryOptions.
//
// RepositoryOptions are part of the Registry struct and set its defaults.
// RepositoryOptions shares the same struct definition as Repository, which
// contains unexported state that must not be copied to multiple Repositories.
// To handle this we explicitly copy only the fields that we want to reproduce.
func newRepositoryWithOptions(ref registry.Reference, opts *RepositoryOptions) (*Repository, error) {
abursavich marked this conversation as resolved.
Show resolved Hide resolved
abursavich marked this conversation as resolved.
Show resolved Hide resolved
if err := ref.ValidateRepository(); err != nil {
return nil, err
}
return &Repository{
Client: opts.Client,
Reference: ref,
PlainHTTP: opts.PlainHTTP,
ManifestMediaTypes: slices.Clone(opts.ManifestMediaTypes),
abursavich marked this conversation as resolved.
Show resolved Hide resolved
TagListPageSize: opts.TagListPageSize,
ReferrerListPageSize: opts.ReferrerListPageSize,
MaxMetadataBytes: opts.MaxMetadataBytes,
}, nil
}

// SetReferrersCapability indicates the Referrers API capability of the remote
// repository. true: capable; false: not capable.
// SetReferrersCapability is valid only when it is called for the first time.
Expand Down