From 70089b39f8973515f6d4e6bcf72842b284a18346 Mon Sep 17 00:00:00 2001 From: David Laubreiter Date: Fri, 17 Mar 2023 11:43:01 +0100 Subject: [PATCH] refactor: Rename ManifestLoaderContext and document symbols Best practice in Go is not to have the package name as a prefix, since it is redundant information in the usage. --- cmd/monaco/delete/delete.go | 2 +- cmd/monaco/deploy/deploy.go | 2 +- cmd/monaco/download/download_configs.go | 2 +- cmd/monaco/download/download_entities.go | 2 +- .../download/download_integration_test.go | 4 ++-- .../integrationtest/integration_test_utils.go | 2 +- .../v1/integration_test_utils.go | 2 +- .../v2/cleanup_all_configs_test.go | 2 +- .../v2/config_restore_e2e_test.go | 2 +- .../v2/integration_test_utils.go | 2 +- cmd/monaco/purge/purge.go | 2 +- cmd/monaco/runner/completion/completion.go | 4 ++-- pkg/manifest/manifest_loader.go | 18 +++++++++++------- pkg/manifest/manifest_loader_test.go | 2 +- 14 files changed, 26 insertions(+), 22 deletions(-) diff --git a/cmd/monaco/delete/delete.go b/cmd/monaco/delete/delete.go index eac8581e8..afbaa7953 100644 --- a/cmd/monaco/delete/delete.go +++ b/cmd/monaco/delete/delete.go @@ -40,7 +40,7 @@ func Delete(fs afero.Fs, deploymentManifestPath string, deleteFile string, envir apis := api.NewAPIs() - manifest, manifestLoadError := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + manifest, manifestLoadError := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: deploymentManifestPath, Environments: environmentNames, diff --git a/cmd/monaco/deploy/deploy.go b/cmd/monaco/deploy/deploy.go index 7c1745206..ebb209134 100644 --- a/cmd/monaco/deploy/deploy.go +++ b/cmd/monaco/deploy/deploy.go @@ -121,7 +121,7 @@ func absPath(manifestPath string) (string, error) { } func loadManifest(fs afero.Fs, manifestPath string, groups []string, environments []string) (*manifest.Manifest, error) { - m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + m, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: manifestPath, Groups: groups, diff --git a/cmd/monaco/download/download_configs.go b/cmd/monaco/download/download_configs.go index c6a6cea7f..f940f29ab 100644 --- a/cmd/monaco/download/download_configs.go +++ b/cmd/monaco/download/download_configs.go @@ -56,7 +56,7 @@ type directDownloadOptions struct { func (d DefaultCommand) DownloadConfigsBasedOnManifest(fs afero.Fs, cmdOptions manifestDownloadOptions) error { - m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + m, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: cmdOptions.manifestFile, }) diff --git a/cmd/monaco/download/download_entities.go b/cmd/monaco/download/download_entities.go index 6cad4692a..028ceb710 100644 --- a/cmd/monaco/download/download_entities.go +++ b/cmd/monaco/download/download_entities.go @@ -45,7 +45,7 @@ type entitiesDirectDownloadOptions struct { func (d DefaultCommand) DownloadEntitiesBasedOnManifest(fs afero.Fs, cmdOptions entitiesManifestDownloadOptions) error { - m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + m, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: cmdOptions.manifestFile, }) diff --git a/cmd/monaco/download/download_integration_test.go b/cmd/monaco/download/download_integration_test.go index 0eeaa0b63..d7f5d89dc 100644 --- a/cmd/monaco/download/download_integration_test.go +++ b/cmd/monaco/download/download_integration_test.go @@ -768,7 +768,7 @@ func TestDownloadIntegrationOverwritesFolderAndManifestIfForced(t *testing.T) { assert.NilError(t, err) // THEN we can load the project again and verify its content - man, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + man, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: filepath.Join(testBasePath, "manifest.yaml"), }) @@ -1013,7 +1013,7 @@ func setupTestingDownloadOptions(t *testing.T, server *httptest.Server, projectN } func loadDownloadedProjects(fs afero.Fs, apis api.APIs) ([]projectLoader.Project, []error) { - man, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + man, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: "out/manifest.yaml", }) diff --git a/cmd/monaco/integrationtest/integration_test_utils.go b/cmd/monaco/integrationtest/integration_test_utils.go index 669d524ca..c4591a2a7 100644 --- a/cmd/monaco/integrationtest/integration_test_utils.go +++ b/cmd/monaco/integrationtest/integration_test_utils.go @@ -46,7 +46,7 @@ func LoadManifest(t *testing.T, fs afero.Fs, manifestFile string, specificEnviro specificEnvs = append(specificEnvs, specificEnvironment) } - m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + m, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: manifestFile, Environments: specificEnvs, diff --git a/cmd/monaco/integrationtest/v1/integration_test_utils.go b/cmd/monaco/integrationtest/v1/integration_test_utils.go index 2953306cd..20e17db4e 100644 --- a/cmd/monaco/integrationtest/v1/integration_test_utils.go +++ b/cmd/monaco/integrationtest/v1/integration_test_utils.go @@ -203,7 +203,7 @@ func runLegacyIntegration(t *testing.T, configFolder, envFile, suffixTest string assert.NilError(t, err) assert.Assert(t, exists, "manifest should exist on path '%s' but does not", manifestPath) - loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: manifestPath, }) diff --git a/cmd/monaco/integrationtest/v2/cleanup_all_configs_test.go b/cmd/monaco/integrationtest/v2/cleanup_all_configs_test.go index e59a89b1f..3ded11854 100644 --- a/cmd/monaco/integrationtest/v2/cleanup_all_configs_test.go +++ b/cmd/monaco/integrationtest/v2/cleanup_all_configs_test.go @@ -37,7 +37,7 @@ func TestDoCleanup(t *testing.T) { manifestPath := "test-resources/test_environments_manifest.yaml" fs := afero.NewOsFs() - loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: manifestPath, }) diff --git a/cmd/monaco/integrationtest/v2/config_restore_e2e_test.go b/cmd/monaco/integrationtest/v2/config_restore_e2e_test.go index 3bf9bb954..6fc6b97ef 100644 --- a/cmd/monaco/integrationtest/v2/config_restore_e2e_test.go +++ b/cmd/monaco/integrationtest/v2/config_restore_e2e_test.go @@ -348,7 +348,7 @@ func validation_uploadDownloadedConfigs(t *testing.T, fs afero.Fs, downloadFolde } func cleanupDeployedConfiguration(t *testing.T, fs afero.Fs, manifestFilepath string, testSuffix string) { - loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: manifestFilepath, }) diff --git a/cmd/monaco/integrationtest/v2/integration_test_utils.go b/cmd/monaco/integrationtest/v2/integration_test_utils.go index 2231b436d..f45a75db2 100644 --- a/cmd/monaco/integrationtest/v2/integration_test_utils.go +++ b/cmd/monaco/integrationtest/v2/integration_test_utils.go @@ -63,7 +63,7 @@ func runIntegrationWithCleanup(t *testing.T, testFs afero.Fs, configFolder, mani envs = append(envs, specificEnvironment) } - loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{ Fs: testFs, ManifestPath: manifestPath, Environments: envs, diff --git a/cmd/monaco/purge/purge.go b/cmd/monaco/purge/purge.go index 8ade13ce2..e6fb71891 100644 --- a/cmd/monaco/purge/purge.go +++ b/cmd/monaco/purge/purge.go @@ -41,7 +41,7 @@ func purge(fs afero.Fs, deploymentManifestPath string, environmentNames []string apis := api.NewAPIs().Filter(api.RetainByName(apiNames)) - mani, manifestLoadError := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + mani, manifestLoadError := manifest.LoadManifest(&manifest.LoaderContext{ Fs: fs, ManifestPath: deploymentManifestPath, Environments: environmentNames, diff --git a/cmd/monaco/runner/completion/completion.go b/cmd/monaco/runner/completion/completion.go index 68828fc08..fa452680f 100644 --- a/cmd/monaco/runner/completion/completion.go +++ b/cmd/monaco/runner/completion/completion.go @@ -111,7 +111,7 @@ func EnvironmentByArg0(_ *cobra.Command, args []string, _ string) ([]string, cob } func loadEnvironmentsFromManifest(manifestPath string) ([]string, cobra.ShellCompDirective) { - man, _ := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + man, _ := manifest.LoadManifest(&manifest.LoaderContext{ Fs: afero.NewOsFs(), ManifestPath: manifestPath, }) @@ -122,7 +122,7 @@ func loadEnvironmentsFromManifest(manifestPath string) ([]string, cobra.ShellCom func ProjectsFromManifest(_ *cobra.Command, args []string, _ string) ([]string, cobra.ShellCompDirective) { manifestPath := args[0] - mani, _ := manifest.LoadManifest(&manifest.ManifestLoaderContext{ + mani, _ := manifest.LoadManifest(&manifest.LoaderContext{ Fs: afero.NewOsFs(), ManifestPath: manifestPath, }) diff --git a/pkg/manifest/manifest_loader.go b/pkg/manifest/manifest_loader.go index b9ea83da6..4c9e3d69f 100644 --- a/pkg/manifest/manifest_loader.go +++ b/pkg/manifest/manifest_loader.go @@ -30,8 +30,12 @@ import ( "strings" ) -type ManifestLoaderContext struct { - Fs afero.Fs +// LoaderContext holds all information for [LoadManifest] +type LoaderContext struct { + // Fs holds the abstraction of the file system. + Fs afero.Fs + + // ManifestPath holds the path from where the manifest should be loaded. ManifestPath string // Environments is a filter to what environments should be loaded. @@ -97,7 +101,7 @@ func (e projectLoaderError) Error() string { return fmt.Sprintf("%s:%s: %s", e.ManifestPath, e.Project, e.Reason) } -func LoadManifest(context *ManifestLoaderContext) (Manifest, []error) { +func LoadManifest(context *LoaderContext) (Manifest, []error) { log.Debug("Loading manifest %q. Restrictions: groups=%q, environments=%q", context.ManifestPath, context.Groups, context.Environments) manifestYAML, err := readManifestYAML(context) @@ -229,7 +233,7 @@ func parseOAuth(a oAuth) (OAuth, error) { }, nil } -func readManifestYAML(context *ManifestLoaderContext) (manifest, error) { +func readManifestYAML(context *LoaderContext) (manifest, error) { manifestPath := filepath.Clean(context.ManifestPath) if !files.IsYamlFileExtension(manifestPath) { @@ -298,7 +302,7 @@ func validateManifestVersion(manifestVersion string) error { return nil } -func toEnvironments(context *ManifestLoaderContext, groups []group) (map[string]EnvironmentDefinition, []error) { // nolint:gocognit +func toEnvironments(context *LoaderContext, groups []group) (map[string]EnvironmentDefinition, []error) { // nolint:gocognit var errors []error environments := make(map[string]EnvironmentDefinition) @@ -366,7 +370,7 @@ func toEnvironments(context *ManifestLoaderContext, groups []group) (map[string] return environments, nil } -func shouldSkipEnv(context *ManifestLoaderContext, group group, env environment) bool { +func shouldSkipEnv(context *LoaderContext, group group, env environment) bool { // if nothing is restricted, everything is allowed if len(context.Groups) == 0 && len(context.Environments) == 0 { return false @@ -383,7 +387,7 @@ func shouldSkipEnv(context *ManifestLoaderContext, group group, env environment) return true } -func parseEnvironment(context *ManifestLoaderContext, config environment, group string) (EnvironmentDefinition, []error) { +func parseEnvironment(context *LoaderContext, config environment, group string) (EnvironmentDefinition, []error) { var errs []error auth, err := parseAuth(config.Auth) diff --git a/pkg/manifest/manifest_loader_test.go b/pkg/manifest/manifest_loader_test.go index 0979c40a6..429953667 100644 --- a/pkg/manifest/manifest_loader_test.go +++ b/pkg/manifest/manifest_loader_test.go @@ -1658,7 +1658,7 @@ environmentGroups: [{name: b, environments: [{name: c, url: {value: d}, auth: {t fs := afero.NewMemMapFs() assert.NoError(t, afero.WriteFile(fs, "manifest.yaml", []byte(test.manifestContent), 0400)) - mani, errs := LoadManifest(&ManifestLoaderContext{ + mani, errs := LoadManifest(&LoaderContext{ Fs: fs, ManifestPath: "manifest.yaml", Groups: test.groups,