From 75cad7d2adfff2d3e2118f9eaa1483b15b161d75 Mon Sep 17 00:00:00 2001 From: user Date: Mon, 23 Feb 2026 11:58:00 -0800 Subject: [PATCH] refactor: add String() methods to domain types, replace string() casts --- internal/docker/client.go | 20 +++++++++---------- internal/domain/types.go | 9 +++++++++ internal/service/deploy/deploy.go | 20 +++++++++---------- .../service/deploy/deploy_container_test.go | 2 +- internal/service/webhook/webhook.go | 4 ++-- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/internal/docker/client.go b/internal/docker/client.go index 6e56b98..626bddb 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -253,7 +253,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID domain.Containe c.log.Info("starting container", "id", containerID) - err := c.docker.ContainerStart(ctx, string(containerID), container.StartOptions{}) + err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{}) if err != nil { return fmt.Errorf("failed to start container: %w", err) } @@ -271,7 +271,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID domain.Container timeout := stopTimeoutSeconds - err := c.docker.ContainerStop(ctx, string(containerID), container.StopOptions{Timeout: &timeout}) + err := c.docker.ContainerStop(ctx, containerID.String(), container.StopOptions{Timeout: &timeout}) if err != nil { return fmt.Errorf("failed to stop container: %w", err) } @@ -291,7 +291,7 @@ func (c *Client) RemoveContainer( c.log.Info("removing container", "id", containerID, "force", force) - err := c.docker.ContainerRemove(ctx, string(containerID), container.RemoveOptions{Force: force}) + err := c.docker.ContainerRemove(ctx, containerID.String(), container.RemoveOptions{Force: force}) if err != nil { return fmt.Errorf("failed to remove container: %w", err) } @@ -315,7 +315,7 @@ func (c *Client) ContainerLogs( Tail: tail, } - reader, err := c.docker.ContainerLogs(ctx, string(containerID), opts) + reader, err := c.docker.ContainerLogs(ctx, containerID.String(), opts) if err != nil { return "", fmt.Errorf("failed to get container logs: %w", err) } @@ -344,7 +344,7 @@ func (c *Client) IsContainerRunning( return false, ErrNotConnected } - inspect, err := c.docker.ContainerInspect(ctx, string(containerID)) + inspect, err := c.docker.ContainerInspect(ctx, containerID.String()) if err != nil { return false, fmt.Errorf("failed to inspect container: %w", err) } @@ -361,7 +361,7 @@ func (c *Client) IsContainerHealthy( return false, ErrNotConnected } - inspect, err := c.docker.ContainerInspect(ctx, string(containerID)) + inspect, err := c.docker.ContainerInspect(ctx, containerID.String()) if err != nil { return false, fmt.Errorf("failed to inspect container: %w", err) } @@ -484,7 +484,7 @@ func (c *Client) CloneRepo( // RemoveImage removes a Docker image by ID or tag. // It returns nil if the image was successfully removed or does not exist. func (c *Client) RemoveImage(ctx context.Context, imageID domain.ImageID) error { - _, err := c.docker.ImageRemove(ctx, string(imageID), image.RemoveOptions{ + _, err := c.docker.ImageRemove(ctx, imageID.String(), image.RemoveOptions{ Force: true, PruneChildren: true, }) @@ -610,7 +610,7 @@ func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResu } defer func() { - _ = c.docker.ContainerRemove(ctx, string(gitContainerID), container.RemoveOptions{Force: true}) + _ = c.docker.ContainerRemove(ctx, gitContainerID.String(), container.RemoveOptions{Force: true}) }() return c.runGitClone(ctx, gitContainerID) @@ -680,12 +680,12 @@ func (c *Client) createGitContainer( } func (c *Client) runGitClone(ctx context.Context, containerID domain.ContainerID) (*CloneResult, error) { - err := c.docker.ContainerStart(ctx, string(containerID), container.StartOptions{}) + err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{}) if err != nil { return nil, fmt.Errorf("failed to start git container: %w", err) } - statusCh, errCh := c.docker.ContainerWait(ctx, string(containerID), container.WaitConditionNotRunning) + statusCh, errCh := c.docker.ContainerWait(ctx, containerID.String(), container.WaitConditionNotRunning) select { case err := <-errCh: diff --git a/internal/domain/types.go b/internal/domain/types.go index 32c3ae8..7811863 100644 --- a/internal/domain/types.go +++ b/internal/domain/types.go @@ -6,11 +6,20 @@ package domain // ImageID is a Docker image identifier (ID or tag). type ImageID string +// String implements the fmt.Stringer interface. +func (id ImageID) String() string { return string(id) } + // ContainerID is a Docker container identifier. type ContainerID string +// String implements the fmt.Stringer interface. +func (id ContainerID) String() string { return string(id) } + // UnparsedURL is a URL stored as a plain string without parsing. // Use this instead of string when the value is known to be a URL // but should not be parsed into a net/url.URL (e.g. webhook URLs, // compare URLs from external payloads). type UnparsedURL string + +// String implements the fmt.Stringer interface. +func (u UnparsedURL) String() string { return string(u) } diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 8ab981c..b25ffc6 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -432,8 +432,8 @@ func (svc *Service) executeRollback( return fmt.Errorf("failed to create rollback container: %w", err) } - deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true} - _ = deployment.AppendLog(bgCtx, "Rollback container created: "+string(containerID)) + deployment.ContainerID = sql.NullString{String: containerID.String(), Valid: true} + _ = deployment.AppendLog(bgCtx, "Rollback container created: "+containerID.String()) startErr := svc.docker.StartContainer(ctx, containerID) if startErr != nil { @@ -696,11 +696,11 @@ func (svc *Service) cleanupCancelledDeploy( if removeErr != nil { svc.log.Error("failed to remove image from cancelled deploy", "error", removeErr, "app", app.Name, "image", imageID) - _ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+string(imageID)+": "+removeErr.Error()) + _ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+imageID.String()+": "+removeErr.Error()) } else { svc.log.Info("cleaned up image from cancelled deploy", "app", app.Name, "image", imageID) - _ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+string(imageID)) + _ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+imageID.String()) } } @@ -851,8 +851,8 @@ func (svc *Service) buildImage( return "", fmt.Errorf("failed to build image: %w", err) } - deployment.ImageID = sql.NullString{String: string(imageID), Valid: true} - _ = deployment.AppendLog(ctx, "Image built: "+string(imageID)) + deployment.ImageID = sql.NullString{String: imageID.String(), Valid: true} + _ = deployment.AppendLog(ctx, "Image built: "+imageID.String()) return imageID, nil } @@ -1039,8 +1039,8 @@ func (svc *Service) createAndStartContainer( return "", fmt.Errorf("failed to create container: %w", err) } - deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true} - _ = deployment.AppendLog(ctx, "Container created: "+string(containerID)) + deployment.ContainerID = sql.NullString{String: containerID.String(), Valid: true} + _ = deployment.AppendLog(ctx, "Container created: "+containerID.String()) startErr := svc.docker.StartContainer(ctx, containerID) if startErr != nil { @@ -1097,7 +1097,7 @@ func (svc *Service) buildContainerOptions( return docker.CreateContainerOptions{ Name: "upaas-" + app.Name, - Image: string(imageID), + Image: imageID.String(), Env: envMap, Labels: buildLabelMap(app, labels), Volumes: buildVolumeMounts(volumes), @@ -1149,7 +1149,7 @@ func (svc *Service) updateAppRunning( app *models.App, imageID domain.ImageID, ) error { - app.ImageID = sql.NullString{String: string(imageID), Valid: true} + app.ImageID = sql.NullString{String: imageID.String(), Valid: true} app.Status = models.AppStatusRunning saveErr := app.Save(ctx) diff --git a/internal/service/deploy/deploy_container_test.go b/internal/service/deploy/deploy_container_test.go index 033d1b8..93b15e4 100644 --- a/internal/service/deploy/deploy_container_test.go +++ b/internal/service/deploy/deploy_container_test.go @@ -35,7 +35,7 @@ func TestBuildContainerOptionsUsesImageID(t *testing.T) { t.Fatalf("buildContainerOptions returned error: %v", err) } - if opts.Image != string(expectedImageID) { + if opts.Image != expectedImageID.String() { t.Errorf("expected Image=%q, got %q", expectedImageID, opts.Image) } diff --git a/internal/service/webhook/webhook.go b/internal/service/webhook/webhook.go index ed7fb33..318e789 100644 --- a/internal/service/webhook/webhook.go +++ b/internal/service/webhook/webhook.go @@ -105,7 +105,7 @@ func (svc *Service) HandleWebhook( event.EventType = eventType event.Branch = branch event.CommitSHA = sql.NullString{String: commitSHA, Valid: commitSHA != ""} - event.CommitURL = sql.NullString{String: string(commitURL), Valid: commitURL != ""} + event.CommitURL = sql.NullString{String: commitURL.String(), Valid: commitURL != ""} event.Payload = sql.NullString{String: string(payload), Valid: true} event.Matched = matched event.Processed = false @@ -179,7 +179,7 @@ func extractCommitURL(payload GiteaPushPayload) domain.UnparsedURL { // Fall back to constructing URL from repo HTML URL if payload.Repository.HTMLURL != "" && payload.After != "" { - return domain.UnparsedURL(string(payload.Repository.HTMLURL) + "/commit/" + payload.After) + return domain.UnparsedURL(payload.Repository.HTMLURL.String() + "/commit/" + payload.After) } return ""