refactor: reorder strategy factory args

internal/cache/api.go

@@ -87,26 +87,28 @@ func FilterTransportHeaders(headers textproto.MIMEHeader) textproto.MIMEHeader {
 }
 
 // A Cache knows how to retrieve, create and delete objects from a cache.
+//
+// Objects in the cache are not guaranteed to persist and implementations may delete them at any time.
 type Cache interface {
 	// String describes the Cache implementation.
 	String() string
 	// Stat returns the headers of an existing object in the cache.
 	//
-	// Expired files SHOULD not be returned.
+	// Expired files MUST not be returned.
 	// Must return os.ErrNotExist if the file does not exist.
 	Stat(ctx context.Context, key Key) (textproto.MIMEHeader, error)
 	// Open an existing file in the cache.
 	//
-	// Expired files SHOULD not be returned.
+	// Expired files MUST NOT be returned.
 	// Must return os.ErrNotExist if the file does not exist.
 	Open(ctx context.Context, key Key) (io.ReadCloser, textproto.MIMEHeader, error)
 	// Create a new file in the cache.
 	//
 	// If "ttl" is zero, a maximum TTL MUST be used by the implementation.
 	//
-	// The file MUST not be available for read until completely written and closed.
+	// The file MUST NOT be available for read until completely written and closed.
 	//
-	// If the context is cancelled the object MUST not be made available in the cache.
+	// If the context is cancelled the object MUST NOT be made available in the cache.
 	Create(ctx context.Context, key Key, headers textproto.MIMEHeader, ttl time.Duration) (io.WriteCloser, error)
 	// Delete a file from the cache.
 	//

internal/cache/remote_test.go

@@ -27,7 +27,7 @@ func TestRemoteClient(t *testing.T) {
 		t.Cleanup(func() { memCache.Close() })
 
 		mux := http.NewServeMux()
-		_, err = strategy.NewAPIV1(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), struct{}{}, memCache, mux)
+		_, err = strategy.NewAPIV1(ctx, struct{}{}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 		assert.NoError(t, err)
 		ts := httptest.NewServer(mux)
 		t.Cleanup(ts.Close)

internal/strategy/api.go

@@ -20,18 +20,18 @@ type Mux interface {
 	HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request))
 }
 
-var registry = map[string]func(ctx context.Context, scheduler jobscheduler.Scheduler, config *hcl.Block, cache cache.Cache, mux Mux) (Strategy, error){}
+var registry = map[string]func(ctx context.Context, config *hcl.Block, scheduler jobscheduler.Scheduler, cache cache.Cache, mux Mux) (Strategy, error){}
 
-type Factory[Config any, S Strategy] func(ctx context.Context, scheduler jobscheduler.Scheduler, config Config, cache cache.Cache, mux Mux) (S, error)
+type Factory[Config any, S Strategy] func(ctx context.Context, config Config, scheduler jobscheduler.Scheduler, cache cache.Cache, mux Mux) (S, error)
 
 // Register a new proxy strategy.
 func Register[Config any, S Strategy](id string, factory Factory[Config, S]) {
-	registry[id] = func(ctx context.Context, scheduler jobscheduler.Scheduler, config *hcl.Block, cache cache.Cache, mux Mux) (Strategy, error) {
+	registry[id] = func(ctx context.Context, config *hcl.Block, scheduler jobscheduler.Scheduler, cache cache.Cache, mux Mux) (Strategy, error) {
 		var cfg Config
 		if err := hcl.UnmarshalBlock(config, &cfg, hcl.AllowExtra(false)); err != nil {
 			return nil, errors.WithStack(err)
 		}
-		return factory(ctx, scheduler, cfg, cache, mux)
+		return factory(ctx, cfg, scheduler, cache, mux)
 	}
 }
 
@@ -47,7 +47,7 @@ func Create(
 	mux Mux,
 ) (Strategy, error) {
 	if factory, ok := registry[name]; ok {
-		return errors.WithStack2(factory(ctx, scheduler.WithQueuePrefix(name), config, cache, mux))
+		return errors.WithStack2(factory(ctx, config, scheduler.WithQueuePrefix(name), cache, mux))
 	}
 	return nil, errors.Errorf("%s: %w", name, ErrNotFound)
 }

internal/strategy/apiv1.go

@@ -28,7 +28,7 @@ type APIV1 struct {
 	logger *slog.Logger
 }
 
-func NewAPIV1(ctx context.Context, _ jobscheduler.Scheduler, _ struct{}, cache cache.Cache, mux Mux) (*APIV1, error) {
+func NewAPIV1(ctx context.Context, _ struct{}, _ jobscheduler.Scheduler, cache cache.Cache, mux Mux) (*APIV1, error) {
 	s := &APIV1{
 		logger: logging.FromContext(ctx),
 		cache:  cache,

internal/strategy/artifactory.go

@@ -53,7 +53,7 @@ type Artifactory struct {
 
 var _ Strategy = (*Artifactory)(nil)
 
-func NewArtifactory(ctx context.Context, _ jobscheduler.Scheduler, config ArtifactoryConfig, cache cache.Cache, mux Mux) (*Artifactory, error) {
+func NewArtifactory(ctx context.Context, config ArtifactoryConfig, _ jobscheduler.Scheduler, cache cache.Cache, mux Mux) (*Artifactory, error) {
 	u, err := url.Parse(config.Target)
 	if err != nil {
 		return nil, fmt.Errorf("invalid target URL: %w", err)

internal/strategy/artifactory_test.go

@@ -64,7 +64,7 @@ func setupArtifactoryTest(t *testing.T, config strategy.ArtifactoryConfig) (*moc
 	t.Cleanup(func() { memCache.Close() })
 
 	mux := http.NewServeMux()
-	_, err = strategy.NewArtifactory(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), config, memCache, mux)
+	_, err = strategy.NewArtifactory(ctx, config, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	return mock, mux, ctx
@@ -211,9 +211,9 @@ func TestArtifactoryString(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	artifactory, err := strategy.NewArtifactory(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.ArtifactoryConfig{
+	artifactory, err := strategy.NewArtifactory(ctx, strategy.ArtifactoryConfig{
 		Target: "https://ec2.example.jfrog.io",
-	}, memCache, mux)
+	}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	assert.Equal(t, "artifactory:ec2.example.jfrog.io", artifactory.String())
@@ -226,9 +226,9 @@ func TestArtifactoryInvalidTargetURL(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	_, err = strategy.NewArtifactory(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.ArtifactoryConfig{
+	_, err = strategy.NewArtifactory(ctx, strategy.ArtifactoryConfig{
 		Target: "://invalid-url",
-	}, memCache, mux)
+	}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.Error(t, err)
 }
 

internal/strategy/git/bundle_test.go

@@ -23,10 +23,10 @@ func TestBundleHTTPEndpoint(t *testing.T) {
 	assert.NoError(t, err)
 	mux := newTestMux()
 
-	_, err = git.New(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), git.Config{
+	_, err = git.New(ctx, git.Config{
 		MirrorRoot:     tmpDir,
 		BundleInterval: 24 * time.Hour,
-	}, memCache, mux)
+	}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	// Create a fake bundle in the cache
@@ -99,10 +99,10 @@ func TestBundleInterval(t *testing.T) {
 			assert.NoError(t, err)
 			mux := newTestMux()
 
-			s, err := git.New(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), git.Config{
+			s, err := git.New(ctx, git.Config{
 				MirrorRoot:     tmpDir,
 				BundleInterval: tt.bundleInterval,
-			}, memCache, mux)
+			}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 			assert.NoError(t, err)
 			assert.NotZero(t, s)
 

internal/strategy/git/git.go

@@ -64,7 +64,7 @@ type Strategy struct {
 	scheduler  jobscheduler.Scheduler
 }
 
-func New(ctx context.Context, scheduler jobscheduler.Scheduler, config Config, cache cache.Cache, mux strategy.Mux) (*Strategy, error) {
+func New(ctx context.Context, config Config, scheduler jobscheduler.Scheduler, cache cache.Cache, mux strategy.Mux) (*Strategy, error) {
 	logger := logging.FromContext(ctx)
 
 	if config.MirrorRoot == "" {

internal/strategy/git/git_test.go

@@ -65,7 +65,7 @@ func TestNew(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mux := newTestMux()
-			s, err := git.New(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), tt.config, nil, mux)
+			s, err := git.New(ctx, tt.config, jobscheduler.New(ctx, jobscheduler.Config{}), nil, mux)
 			if tt.wantError != "" {
 				assert.Error(t, err)
 				assert.Contains(t, err.Error(), tt.wantError)
@@ -145,10 +145,10 @@ func TestNewWithExistingCloneOnDisk(t *testing.T) {
 	assert.NoError(t, err)
 
 	mux := newTestMux()
-	s, err := git.New(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), git.Config{
+	s, err := git.New(ctx, git.Config{
 		MirrorRoot:    tmpDir,
 		FetchInterval: 15,
-	}, nil, mux)
+	}, jobscheduler.New(ctx, jobscheduler.Config{}), nil, mux)
 	assert.NoError(t, err)
 	assert.NotZero(t, s)
 }
@@ -168,10 +168,10 @@ func TestIntegrationWithMockUpstream(t *testing.T) {
 
 	// Create strategy - it will register handlers
 	mux := newTestMux()
-	_, err := git.New(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), git.Config{
+	_, err := git.New(ctx, git.Config{
 		MirrorRoot:    tmpDir,
 		FetchInterval: 15,
-	}, nil, mux)
+	}, jobscheduler.New(ctx, jobscheduler.Config{}), nil, mux)
 	assert.NoError(t, err)
 
 	// Verify handlers exist

internal/strategy/github_releases.go

@@ -34,7 +34,7 @@ type GitHubReleases struct {
 }
 
 // NewGitHubReleases creates a [Strategy] that fetches private (and public) release binaries from GitHub.
-func NewGitHubReleases(ctx context.Context, _ jobscheduler.Scheduler, config GitHubReleasesConfig, cache cache.Cache, mux Mux) (*GitHubReleases, error) {
+func NewGitHubReleases(ctx context.Context, config GitHubReleasesConfig, _ jobscheduler.Scheduler, cache cache.Cache, mux Mux) (*GitHubReleases, error) {
 	s := &GitHubReleases{
 		config: config,
 		cache:  cache,

internal/strategy/github_releases_test.go

@@ -127,7 +127,7 @@ func setupTest(t *testing.T, config strategy.GitHubReleasesConfig) (*mockGitHubS
 	t.Cleanup(func() { memCache.Close() })
 
 	mux := http.NewServeMux()
-	_, err = strategy.NewGitHubReleases(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), config, memCache, mux)
+	_, err = strategy.NewGitHubReleases(ctx, config, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	return mock, mux, ctx
@@ -242,7 +242,7 @@ func TestGitHubReleasesNoToken(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	gh, err := strategy.NewGitHubReleases(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.GitHubReleasesConfig{}, memCache, mux)
+	gh, err := strategy.NewGitHubReleases(ctx, strategy.GitHubReleasesConfig{}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 	assert.Equal(t, "github-releases", gh.String())
 }
@@ -254,9 +254,9 @@ func TestGitHubReleasesString(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	gh, err := strategy.NewGitHubReleases(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.GitHubReleasesConfig{
+	gh, err := strategy.NewGitHubReleases(ctx, strategy.GitHubReleasesConfig{
 		Token: "test-token",
-	}, memCache, mux)
+	}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	assert.Equal(t, "github-releases", gh.String())

internal/strategy/host.go

@@ -41,7 +41,7 @@ type Host struct {
 
 var _ Strategy = (*Host)(nil)
 
-func NewHost(ctx context.Context, _ jobscheduler.Scheduler, config HostConfig, cache cache.Cache, mux Mux) (*Host, error) {
+func NewHost(ctx context.Context, config HostConfig, _ jobscheduler.Scheduler, cache cache.Cache, mux Mux) (*Host, error) {
 	u, err := url.Parse(config.Target)
 	if err != nil {
 		return nil, fmt.Errorf("invalid target URL: %w", err)

internal/strategy/host_test.go

@@ -32,7 +32,7 @@ func TestHostCaching(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	_, err = strategy.NewHost(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.HostConfig{Target: backend.URL}, memCache, mux)
+	_, err = strategy.NewHost(ctx, strategy.HostConfig{Target: backend.URL}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	// Request path must include the host prefix from the target URL
@@ -69,7 +69,7 @@ func TestHostNonOKStatus(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	_, err = strategy.NewHost(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.HostConfig{Target: backend.URL}, memCache, mux)
+	_, err = strategy.NewHost(ctx, strategy.HostConfig{Target: backend.URL}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	// Request path must include the host prefix from the target URL
@@ -95,7 +95,7 @@ func TestHostInvalidTargetURL(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	_, err = strategy.NewHost(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.HostConfig{Target: "://invalid"}, memCache, mux)
+	_, err = strategy.NewHost(ctx, strategy.HostConfig{Target: "://invalid"}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.Error(t, err)
 }
 
@@ -106,7 +106,7 @@ func TestHostString(t *testing.T) {
 	defer memCache.Close()
 
 	mux := http.NewServeMux()
-	host, err := strategy.NewHost(ctx, jobscheduler.New(ctx, jobscheduler.Config{}), strategy.HostConfig{Target: "https://example.com/prefix"}, memCache, mux)
+	host, err := strategy.NewHost(ctx, strategy.HostConfig{Target: "https://example.com/prefix"}, jobscheduler.New(ctx, jobscheduler.Config{}), memCache, mux)
 	assert.NoError(t, err)
 
 	assert.Equal(t, "host:example.com/prefix", host.String())