Move bundle/run.runPageURL to libs/workspaceurls

bundle/run/job.go

@@ -5,9 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"net/url"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/databricks/cli/bundle"
@@ -99,7 +97,7 @@ func (m *jobRunMonitor) onProgress(info *jobs.Run) {
 
 	// First time we see this run.
 	if m.prevState == nil {
-		runURL := runPageURL(m.ctx, info.RunPageUrl)
+		runURL := workspaceurls.ModernizeJobRunPageURL(info.RunPageUrl)
 		log.Infof(m.ctx, "Run available at %s", runURL)
 		cmdio.Log(m.ctx, progress.NewJobRunUrlEvent(runURL))
 	}
@@ -126,47 +124,6 @@ func (m *jobRunMonitor) onProgress(info *jobs.Run) {
 	log.Info(m.ctx, event.String())
 }
 
-// runPageURL converts the legacy run URL returned by the Jobs API
-//
-//	https://<host>/?o=<id>#job/<jobID>/run/<runID>
-//
-// into the modern path form
-//
-//	https://<host>/jobs/<jobID>/runs/<runID>?o=<id>
-//
-// so that non-admin users permitted to view the run are not redirected to the
-// workspace homepage. See https://github.com/databricks/cli/issues/5142. The
-// workspace selector query param (o) is preserved as-is. The conversion is
-// cosmetic, so the original URL is returned on the rare chance the format is
-// unexpected.
-func runPageURL(ctx context.Context, raw string) string {
-	u, err := url.Parse(raw)
-	if err != nil {
-		log.Debugf(ctx, "could not parse run URL %q: %v", raw, err)
-		return raw
-	}
-
-	jobID, runID, ok := parseLegacyRunFragment(u.Fragment)
-	if !ok {
-		log.Debugf(ctx, "unexpected run URL fragment %q", u.Fragment)
-		return raw
-	}
-
-	u.Fragment = ""
-	u.Path = "/" + workspaceurls.JobRunPath(jobID, runID)
-	return u.String()
-}
-
-// parseLegacyRunFragment extracts the job and run IDs from a legacy run URL
-// fragment of the form "job/<jobID>/run/<runID>".
-func parseLegacyRunFragment(fragment string) (jobID, runID string, ok bool) {
-	parts := strings.Split(fragment, "/")
-	if len(parts) != 4 || parts[0] != "job" || parts[2] != "run" || parts[1] == "" || parts[3] == "" {
-		return "", "", false
-	}
-	return parts[1], parts[3], true
-}
-
 func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, error) {
 	jobID, err := strconv.ParseInt(r.job.ID, 10, 64)
 	if err != nil {
@@ -205,7 +162,7 @@ func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e
 		if err != nil {
 			return nil, err
 		}
-		cmdio.Log(ctx, progress.NewJobRunUrlEvent(runPageURL(ctx, details.RunPageUrl)))
+		cmdio.Log(ctx, progress.NewJobRunUrlEvent(workspaceurls.ModernizeJobRunPageURL(details.RunPageUrl)))
 		return nil, nil
 	}
 

bundle/run/job_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
@@ -292,50 +291,3 @@ func TestJobRunnerRestartForContinuousUnpausedJobs(t *testing.T) {
 	_, err := runner.Restart(ctx, &Options{})
 	require.NoError(t, err)
 }
-
-func TestRunPageURL(t *testing.T) {
-	ctx := t.Context()
-	tests := []struct {
-		name     string
-		raw      string
-		expected string
-	}{
-		{
-			"legacy fragment form preserves workspace selector",
-			"https://myworkspace.databricks.test/?o=900800700600#job/123/run/456",
-			"https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600",
-		},
-		{
-			"no workspace selector",
-			"https://myworkspace.databricks.test/#job/123/run/456",
-			"https://myworkspace.databricks.test/jobs/123/runs/456",
-		},
-		{
-			"http host with port",
-			"http://127.0.0.1:8080/?o=900800700600#job/1/run/2",
-			"http://127.0.0.1:8080/jobs/1/runs/2?o=900800700600",
-		},
-		// Unexpected formats are returned unchanged because the conversion is cosmetic.
-		{
-			"already modern path is left as-is",
-			"https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600",
-			"https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600",
-		},
-		{
-			"incomplete fragment is left as-is",
-			"https://myworkspace.databricks.test/?o=900800700600#job/123",
-			"https://myworkspace.databricks.test/?o=900800700600#job/123",
-		},
-		{
-			"empty job id is left as-is",
-			"https://myworkspace.databricks.test/?o=900800700600#job//run/456",
-			"https://myworkspace.databricks.test/?o=900800700600#job//run/456",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, tt.expected, runPageURL(ctx, tt.raw))
-		})
-	}
-}

libs/workspaceurls/urls.go

@@ -90,6 +90,45 @@ func JobRunURL(baseURL url.URL, jobID, runID string) string {
 	return baseURL.String()
 }
 
+// ModernizeJobRunPageURL converts the legacy run URL returned by the Jobs API
+//
+//	https://<host>/?o=<id>#job/<jobID>/run/<runID>
+//
+// into the modern path form
+//
+//	https://<host>/jobs/<jobID>/runs/<runID>?o=<id>
+//
+// so that non-admin users permitted to view the run are not redirected to the
+// workspace homepage. See https://github.com/databricks/cli/issues/5142. The
+// workspace selector query param (o) is preserved as-is. The conversion is
+// cosmetic, so the original URL is returned on the rare chance the format is
+// unexpected.
+func ModernizeJobRunPageURL(raw string) string {
+	u, err := url.Parse(raw)
+	if err != nil {
+		return raw
+	}
+
+	jobID, runID, ok := parseLegacyRunFragment(u.Fragment)
+	if !ok {
+		return raw
+	}
+
+	u.Fragment = ""
+	u.Path = "/" + JobRunPath(jobID, runID)
+	return u.String()
+}
+
+// parseLegacyRunFragment extracts the job and run IDs from a legacy run URL
+// fragment of the form "job/<jobID>/run/<runID>".
+func parseLegacyRunFragment(fragment string) (jobID, runID string, ok bool) {
+	parts := strings.Split(fragment, "/")
+	if len(parts) != 4 || parts[0] != "job" || parts[2] != "run" || parts[1] == "" || parts[3] == "" {
+		return "", "", false
+	}
+	return parts[1], parts[3], true
+}
+
 // ResourceURL constructs a workspace URL for a named resource type and ID.
 func ResourceURL(baseURL url.URL, resourceType, id string) string {
 	resourceType = resolveAlias(resourceType)

libs/workspaceurls/urls_test.go

@@ -166,3 +166,49 @@ func TestHasWorkspaceIDInHostname(t *testing.T) {
 		})
 	}
 }
+
+func TestModernizeJobRunPageURL(t *testing.T) {
+	tests := []struct {
+		name     string
+		raw      string
+		expected string
+	}{
+		{
+			"legacy fragment form preserves workspace selector",
+			"https://myworkspace.databricks.test/?o=900800700600#job/123/run/456",
+			"https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600",
+		},
+		{
+			"no workspace selector",
+			"https://myworkspace.databricks.test/#job/123/run/456",
+			"https://myworkspace.databricks.test/jobs/123/runs/456",
+		},
+		{
+			"http host with port",
+			"http://127.0.0.1:8080/?o=900800700600#job/1/run/2",
+			"http://127.0.0.1:8080/jobs/1/runs/2?o=900800700600",
+		},
+		// The conversion is cosmetic, so any other format passes through unchanged.
+		{
+			"already modern path is left as-is",
+			"https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600",
+			"https://myworkspace.databricks.test/jobs/123/runs/456?o=900800700600",
+		},
+		{
+			"incomplete fragment is left as-is",
+			"https://myworkspace.databricks.test/?o=900800700600#job/123",
+			"https://myworkspace.databricks.test/?o=900800700600#job/123",
+		},
+		{
+			"empty job id is left as-is",
+			"https://myworkspace.databricks.test/?o=900800700600#job//run/456",
+			"https://myworkspace.databricks.test/?o=900800700600#job//run/456",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, ModernizeJobRunPageURL(tt.raw))
+		})
+	}
+}