From 31ecf65e44c849b015e7ea6c338a11eddbdec6db Mon Sep 17 00:00:00 2001 From: Jack McCluskey Date: Wed, 2 Mar 2022 19:43:41 +0000 Subject: [PATCH 1/2] [BEAM-14029] Add getter, setter for target maven repo --- .../runtime/xlangx/expansionx/download.go | 38 ++++++++++- .../xlangx/expansionx/download_test.go | 66 +++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go index 0f794668d87e..7166fbec6023 100644 --- a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go +++ b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go @@ -50,6 +50,24 @@ func newJarGetter() *jarGetter { return &jarGetter{repository: apacheRepository, groupID: beamGroupID, jarCache: cacheDir} } +// GetRepositoryURL returns the current target URL for the defaultJarGetter, +// indicating what repository will be connected to when getting a Beam JAR. +func GetDefaultRepositoryURL() string { + return defaultJarGetter.GetRepositoryURL() +} + +// SetRepositoryURL updates the target URL for the defaultJarGetter, changing +// which Maven repository will be connected to when getting a Beam JAR. Also +// validates that it has been passed a URL and returns an error if not. +// +// When changing the target repository, make sure that the value is the prefix +// up to "org/apache/beam" and that the organization of the repository matches +// that of the default from that point on to ensure that the conversion of the +// Gradle target to the JAR name is correct. +func SetDefaultRepositoryURL(repoURL string) error { + return defaultJarGetter.SetRepositoryURL(repoURL) +} + // GetBeamJar checks a temporary directory for the desired Beam JAR, downloads the // appropriate JAR from Maven if not present, then returns the file path to the // JAR. @@ -57,6 +75,24 @@ func GetBeamJar(gradleTarget, version string) (string, error) { return defaultJarGetter.getJar(gradleTarget, version) } +// GetRepositoryURL returns the current target URL for the jarGetter, +// indicating what repository will be connected to when getting a Beam JAR. +func (j *jarGetter) GetRepositoryURL() string { + return string(j.repository) +} + +// SetRepositoryURL updates the target URL for the jarGetter, changing +// which Maven repository will be connected to when getting a Beam JAR. Also +// does some minor validation that it has been passed a URL and returns an +// error if not. +func (j *jarGetter) SetRepositoryURL(repoURL string) error { + if !strings.HasPrefix(repoURL, "http") { + return fmt.Errorf("repo URL %v does not have an http or https prefix", repoURL) + } + j.repository = url(strings.TrimSuffix(repoURL, "/")) + return nil +} + func (j *jarGetter) getJar(gradleTarget, version string) (string, error) { strippedTarget := dropEndOfGradleTarget(gradleTarget) fullURL, jarName := j.getURLForBeamJar(strippedTarget, version) @@ -84,7 +120,7 @@ func (j *jarGetter) getJar(gradleTarget, version string) (string, error) { defer resp.Body.Close() if resp.StatusCode != 200 { - return "", fmt.Errorf("received non 200 response code, got %v", resp.StatusCode) + return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", fullURL, resp.StatusCode) } file, err := os.Create(jarPath) diff --git a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go index 1d13faca6bbf..3960dc95dd87 100644 --- a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go +++ b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go @@ -23,6 +23,72 @@ import ( "testing" ) +func TestGetAndSetRepositoryURL(t *testing.T) { + tests := []struct { + name string + newRepo string + expRepo string + }{ + { + "correct URL", + "http://new.repo.org", + "http://new.repo.org", + }, + { + "correct URL https", + "https://new.repo.org", + "https://new.repo.org", + }, + { + "correct URL with trailing backslash", + "http://new.repo.org/", + "http://new.repo.org", + }, + { + "correct URL https with trailing backslash", + "https://new.repo.org/", + "https://new.repo.org", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + j := newJarGetter() + err := j.SetRepositoryURL(test.newRepo) + if err != nil { + t.Errorf("failed to set repository URL, got %v", err) + } + if got, want := j.GetRepositoryURL(), test.expRepo; got != want { + t.Errorf("GetRepositoryURL() got %v, want %v", got, want) + } + }) + } +} + +func TestGetAndSetRepositoryURL_bad(t *testing.T) { + tests := []struct { + name string + newRepo string + }{ + { + "no http", + "new.maven.repo.com", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + j := newJarGetter() + err := j.SetRepositoryURL(test.newRepo) + if err == nil { + t.Errorf("SetRepositoryURL(%v) succeeded when it should have failed", test.newRepo) + } + // Check that the failed Set call did not change the URL. + if got, want := j.GetRepositoryURL(), string(apacheRepository); got != want { + t.Errorf("GetRepositoryURL() got %v, want %v", got, want) + } + }) + } +} + func TestGetURLForBeamJar(t *testing.T) { tests := []struct { name string From 923f860fd0b50412e2391f6f6e916ee89eee7715 Mon Sep 17 00:00:00 2001 From: Jack McCluskey Date: Wed, 2 Mar 2022 20:40:58 +0000 Subject: [PATCH 2/2] make internal get/set implementation unexported --- .../core/runtime/xlangx/expansionx/download.go | 14 ++++---------- .../runtime/xlangx/expansionx/download_test.go | 14 +++++++------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go index 7166fbec6023..59a1d575248b 100644 --- a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go +++ b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go @@ -53,7 +53,7 @@ func newJarGetter() *jarGetter { // GetRepositoryURL returns the current target URL for the defaultJarGetter, // indicating what repository will be connected to when getting a Beam JAR. func GetDefaultRepositoryURL() string { - return defaultJarGetter.GetRepositoryURL() + return defaultJarGetter.getRepositoryURL() } // SetRepositoryURL updates the target URL for the defaultJarGetter, changing @@ -65,7 +65,7 @@ func GetDefaultRepositoryURL() string { // that of the default from that point on to ensure that the conversion of the // Gradle target to the JAR name is correct. func SetDefaultRepositoryURL(repoURL string) error { - return defaultJarGetter.SetRepositoryURL(repoURL) + return defaultJarGetter.setRepositoryURL(repoURL) } // GetBeamJar checks a temporary directory for the desired Beam JAR, downloads the @@ -75,17 +75,11 @@ func GetBeamJar(gradleTarget, version string) (string, error) { return defaultJarGetter.getJar(gradleTarget, version) } -// GetRepositoryURL returns the current target URL for the jarGetter, -// indicating what repository will be connected to when getting a Beam JAR. -func (j *jarGetter) GetRepositoryURL() string { +func (j *jarGetter) getRepositoryURL() string { return string(j.repository) } -// SetRepositoryURL updates the target URL for the jarGetter, changing -// which Maven repository will be connected to when getting a Beam JAR. Also -// does some minor validation that it has been passed a URL and returns an -// error if not. -func (j *jarGetter) SetRepositoryURL(repoURL string) error { +func (j *jarGetter) setRepositoryURL(repoURL string) error { if !strings.HasPrefix(repoURL, "http") { return fmt.Errorf("repo URL %v does not have an http or https prefix", repoURL) } diff --git a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go index 3960dc95dd87..b20977b5e704 100644 --- a/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go +++ b/sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download_test.go @@ -53,12 +53,12 @@ func TestGetAndSetRepositoryURL(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { j := newJarGetter() - err := j.SetRepositoryURL(test.newRepo) + err := j.setRepositoryURL(test.newRepo) if err != nil { t.Errorf("failed to set repository URL, got %v", err) } - if got, want := j.GetRepositoryURL(), test.expRepo; got != want { - t.Errorf("GetRepositoryURL() got %v, want %v", got, want) + if got, want := j.getRepositoryURL(), test.expRepo; got != want { + t.Errorf("getRepositoryURL() got %v, want %v", got, want) } }) } @@ -77,13 +77,13 @@ func TestGetAndSetRepositoryURL_bad(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { j := newJarGetter() - err := j.SetRepositoryURL(test.newRepo) + err := j.setRepositoryURL(test.newRepo) if err == nil { - t.Errorf("SetRepositoryURL(%v) succeeded when it should have failed", test.newRepo) + t.Errorf("setRepositoryURL(%v) succeeded when it should have failed", test.newRepo) } // Check that the failed Set call did not change the URL. - if got, want := j.GetRepositoryURL(), string(apacheRepository); got != want { - t.Errorf("GetRepositoryURL() got %v, want %v", got, want) + if got, want := j.getRepositoryURL(), string(apacheRepository); got != want { + t.Errorf("getRepositoryURL() got %v, want %v", got, want) } }) }