Skip to content

Commit

Permalink
Objectify configuration
Browse files Browse the repository at this point in the history
We can now create configuration snapshots, instead of retrieving the
different configuration properties individually.

Pros:
- Warnings about custom configurations are printed at the beginning of
  each command, instead of whenever commands ask for them.
- Warnings about custom configurations are printed only once even when
  commands ask for them repeatedly.
  • Loading branch information
nictas committed Feb 26, 2020
1 parent 0000770 commit ae17597
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 61 deletions.
8 changes: 5 additions & 3 deletions commands/base_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type BaseCommand struct {
clientFactory clients.ClientFactory
tokenFactory baseclient.TokenFactory
deployServiceURLCalculator util.DeployServiceURLCalculator
configurationSnapshot configuration.Snapshot
}

// Initialize initializes the command with the specified name and CLI connection
Expand All @@ -59,19 +60,20 @@ func (c *BaseCommand) Initialize(name string, cliConnection plugin.CliConnection
jar := newCookieJar()
tokenFactory := NewDefaultTokenFactory(cliConnection)
cloudFoundryClient := cfrestclient.NewCloudFoundryRestClient(getApiEndpoint(cliConnection), transport, jar, tokenFactory)
c.InitializeAll(name, cliConnection, transport, jar, clients.NewDefaultClientFactory(), tokenFactory, util.NewDeployServiceURLCalculator(cloudFoundryClient))
c.InitializeAll(name, cliConnection, transport, jar, clients.NewDefaultClientFactory(), tokenFactory, util.NewDeployServiceURLCalculator(cloudFoundryClient), configuration.NewSnapshot())
}

// InitializeAll initializes the command with the specified name, CLI connection, transport and cookie jar.
func (c *BaseCommand) InitializeAll(name string, cliConnection plugin.CliConnection,
transport http.RoundTripper, jar http.CookieJar, clientFactory clients.ClientFactory, tokenFactory baseclient.TokenFactory, deployServiceURLCalculator util.DeployServiceURLCalculator) {
transport http.RoundTripper, jar http.CookieJar, clientFactory clients.ClientFactory, tokenFactory baseclient.TokenFactory, deployServiceURLCalculator util.DeployServiceURLCalculator, configurationSnapshot configuration.Snapshot) {
c.name = name
c.cliConnection = cliConnection
c.transport = transport
c.jar = jar
c.clientFactory = clientFactory
c.tokenFactory = tokenFactory
c.deployServiceURLCalculator = deployServiceURLCalculator
c.configurationSnapshot = configurationSnapshot
}

func getApiEndpoint(cliConnection plugin.CliConnection) string {
Expand Down Expand Up @@ -221,7 +223,7 @@ func (c *BaseCommand) GetCustomDeployServiceURL(args []string) string {
ui.Say(fmt.Sprintf("**Attention: You've specified a custom Deploy Service URL (%s) via the command line option 'u'. The application listening on that URL may be outdated, contain bugs or unreleased features or may even be modified by a potentially untrused person. Use at your own risk.**\n", optionDeployServiceURL))
return optionDeployServiceURL
}
return configuration.GetBackendURL()
return c.configurationSnapshot.GetBackendURL()
}

// ExecuteAction executes the action over the process specified with operationID
Expand Down
5 changes: 3 additions & 2 deletions commands/base_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
mtafake "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient/fakes"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/restclient/fakes"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/testutil"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
util_fakes "github.com/cloudfoundry-incubator/multiapps-cli-plugin/util/fakes"
Expand Down Expand Up @@ -126,7 +127,7 @@ var _ = Describe("BaseCommand", func() {
Build()
deployServiceURLCalculator := util_fakes.NewDeployServiceURLFakeCalculator("deploy-service.test.ondemand.com")

command.InitializeAll("test", fakeCliConnection, testutil.NewCustomTransport(http.StatusOK, nil), nil, testClientFactory, testTokenFactory, deployServiceURLCalculator)
command.InitializeAll("test", fakeCliConnection, testutil.NewCustomTransport(http.StatusOK, nil), nil, testClientFactory, testTokenFactory, deployServiceURLCalculator, configuration.NewSnapshot())
})
Context("with valid ongoing operations", func() {
It("should abort and exit with zero status", func() {
Expand Down Expand Up @@ -184,7 +185,7 @@ var _ = Describe("BaseCommand", func() {
ExecuteAction("test-process-id", "abort", mtaclient.ResponseHeader{}, nil).
ExecuteAction("test-process-id", "retry", mtaclient.ResponseHeader{Location: "operations/test-process-id?embed=messages"}, nil).Build()
deployServiceURLCalculator := util_fakes.NewDeployServiceURLFakeCalculator("deploy-service.test.ondemand.com")
command.InitializeAll("test", fakeCliConnection, testutil.NewCustomTransport(200, nil), nil, testClientfactory, testTokenFactory, deployServiceURLCalculator)
command.InitializeAll("test", fakeCliConnection, testutil.NewCustomTransport(200, nil), nil, testClientfactory, testTokenFactory, deployServiceURLCalculator, configuration.NewSnapshot())
})
Context("with valid process id and valid action id", func() {
It("should abort and exit with zero status", func() {
Expand Down
31 changes: 16 additions & 15 deletions commands/deploy_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,17 @@ func (c *DeployCommand) GetPluginCommand() plugin.Command {
Perform action on an active deploy operation
cf deploy -i OPERATION_ID -a ACTION [-u URL]`,
Options: map[string]string{
extDescriptorsOpt: "Extension descriptors",
deployServiceURLOpt: "Deploy service URL, by default 'deploy-service.<system-domain>'",
timeoutOpt: "Start timeout in seconds",
versionRuleOpt: "Version rule (HIGHER, SAME_HIGHER, ALL)",
operationIDOpt: "Active deploy operation id",
actionOpt: "Action to perform on active deploy operation (abort, retry, monitor)",
forceOpt: "Force deploy without confirmation for aborting conflicting processes",
moduleOpt: "Deploy list of modules which are contained in the deployment descriptor, in the current location",
resourceOpt: "Deploy list of resources which are contained in the deployment descriptor, in the current location",
util.GetShortOption(noStartOpt): "Do not start apps",
util.GetShortOption(useNamespacesOpt): "Use namespaces in app and service names",
extDescriptorsOpt: "Extension descriptors",
deployServiceURLOpt: "Deploy service URL, by default 'deploy-service.<system-domain>'",
timeoutOpt: "Start timeout in seconds",
versionRuleOpt: "Version rule (HIGHER, SAME_HIGHER, ALL)",
operationIDOpt: "Active deploy operation id",
actionOpt: "Action to perform on active deploy operation (abort, retry, monitor)",
forceOpt: "Force deploy without confirmation for aborting conflicting processes",
moduleOpt: "Deploy list of modules which are contained in the deployment descriptor, in the current location",
resourceOpt: "Deploy list of resources which are contained in the deployment descriptor, in the current location",
util.GetShortOption(noStartOpt): "Do not start apps",
util.GetShortOption(useNamespacesOpt): "Use namespaces in app and service names",
util.GetShortOption(noNamespacesForServicesOpt): "Do not use namespaces in service names",
util.GetShortOption(deleteServicesOpt): "Recreate changed services / delete discontinued services",
util.GetShortOption(deleteServiceKeysOpt): "Delete existing service keys and apply the new ones",
Expand Down Expand Up @@ -286,8 +286,9 @@ func (c *DeployCommand) Execute(args []string) ExecutionStatus {
return Failure
}

chunkSizeInMB := c.configurationSnapshot.GetChunkSizeInMB()
// Upload the MTA archive file
mtaArchiveUploader := NewFileUploader([]string{mtaArchivePath}, mtaClient)
mtaArchiveUploader := NewFileUploader([]string{mtaArchivePath}, mtaClient, chunkSizeInMB)
uploadedMtaArchives, status := mtaArchiveUploader.UploadFiles()
if status == Failure {
return Failure
Expand All @@ -300,7 +301,7 @@ func (c *DeployCommand) Execute(args []string) ExecutionStatus {
// Upload the extension descriptor files
var uploadedExtDescriptorIDs []string
if len(extDescriptorPaths) != 0 {
extDescriptorsUploader := NewFileUploader(extDescriptorPaths, mtaClient)
extDescriptorsUploader := NewFileUploader(extDescriptorPaths, mtaClient, chunkSizeInMB)
uploadedExtDescriptors, status := extDescriptorsUploader.UploadFiles()
if status == Failure {
return Failure
Expand Down Expand Up @@ -487,13 +488,13 @@ func determinePositionalArgumentsTovalidate(possitionalArgument string) []string
return []string{"MTA"}
}

type deployCommandFlagsValidator struct {}
type deployCommandFlagsValidator struct{}

func (deployCommandFlagsValidator) ValidateParsedFlags(flags *flag.FlagSet) error {
var err error
flags.Visit(func(f *flag.Flag) {
if f.Name == strategyOpt {
if f.Value.String() == "" {
if f.Value.String() == "" {
err = errors.New("strategy flag defined but no argument specified")
} else if !util.Contains(AvailableStrategies(), f.Value.String()) {
err = fmt.Errorf("%s is not a valid deployment strategy, available strategies: %v", f.Value.String(), AvailableStrategies())
Expand Down
3 changes: 2 additions & 1 deletion commands/deploy_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
mtafake "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient/fakes"

"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/testutil"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/util"
Expand Down Expand Up @@ -130,7 +131,7 @@ var _ = Describe("DeployCommand", func() {
command = commands.NewDeployCommand()
testTokenFactory := commands.NewTestTokenFactory(cliConnection)
deployServiceURLCalculator := util_fakes.NewDeployServiceURLFakeCalculator("deploy-service.test.ondemand.com")
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, testClientFactory, testTokenFactory, deployServiceURLCalculator)
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, testClientFactory, testTokenFactory, deployServiceURLCalculator, configuration.NewSnapshot())
})

// unknown flag - error
Expand Down
3 changes: 2 additions & 1 deletion commands/download_mta_op_logs_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models"
mtafake "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient/fakes"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/testutil"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
util_fakes "github.com/cloudfoundry-incubator/multiapps-cli-plugin/util/fakes"
Expand Down Expand Up @@ -67,7 +68,7 @@ var _ = Describe("DownloadMtaOperationLogsCommand", func() {
command = &commands.DownloadMtaOperationLogsCommand{}
testTokenFactory := commands.NewTestTokenFactory(cliConnection)
deployServiceURLCalculator := util_fakes.NewDeployServiceURLFakeCalculator("deploy-service.test.ondemand.com")
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, clientFactory, testTokenFactory, deployServiceURLCalculator)
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, clientFactory, testTokenFactory, deployServiceURLCalculator, configuration.NewSnapshot())
})

// unknown flag - error
Expand Down
16 changes: 8 additions & 8 deletions commands/file_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/baseclient"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/log"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/util"
Expand All @@ -21,15 +20,17 @@ import (

//FileUploader uploads files for the service with the specified service ID
type FileUploader struct {
files []string
mtaClient mtaclient.MtaClientOperations
files []string
mtaClient mtaclient.MtaClientOperations
chunkSizeInMB uint64
}

//NewFileUploader creates a new file uploader for the specified service ID, files, and SLMP client
func NewFileUploader(files []string, mtaClient mtaclient.MtaClientOperations) *FileUploader {
func NewFileUploader(files []string, mtaClient mtaclient.MtaClientOperations, chunkSizeInMB uint64) *FileUploader {
return &FileUploader{
files: files,
mtaClient: mtaClient,
files: files,
mtaClient: mtaClient,
chunkSizeInMB: chunkSizeInMB,
}
}

Expand Down Expand Up @@ -75,7 +76,6 @@ func (f *FileUploader) UploadFiles() ([]*models.FileMetadata, ExecutionStatus) {
var uploadedFiles []*models.FileMetadata
uploadedFiles = append(uploadedFiles, alreadyUploadedFiles...)
if len(filesToUpload) != 0 {
chunkSizeInMB := configuration.GetChunkSizeInMB()
ui.Say("Uploading %d files...", len(filesToUpload))

// Iterate over all files to be uploaded
Expand All @@ -89,7 +89,7 @@ func (f *FileUploader) UploadFiles() ([]*models.FileMetadata, ExecutionStatus) {
ui.Say(" " + fullPath)

// Upload the file
uploaded, err := uploadInChunks(fullPath, fileToUpload, chunkSizeInMB, f.mtaClient)
uploaded, err := uploadInChunks(fullPath, fileToUpload, f.chunkSizeInMB, f.mtaClient)
if err != nil {
ui.Failed("Could not upload file %s: %s", terminal.EntityNameColor(fileToUpload.Name()), err.Error())
return nil, Failure
Expand Down
13 changes: 7 additions & 6 deletions commands/file_uploader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient/fakes"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/testutil"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/util"
Expand Down Expand Up @@ -43,7 +44,7 @@ var _ = Describe("FileUploader", func() {
client := fakeSlmpClientBuilder.GetMtaFiles([]*models.FileMetadata{}, nil).Build()

output := oc.CaptureOutput(func() {
fileUploader = commands.NewFileUploader([]string{}, client)
fileUploader = commands.NewFileUploader([]string{}, client, configuration.DefaultChunkSizeInMB)
uploadedFiles, status = fileUploader.UploadFiles()
})
ex.ExpectSuccess(status.ToInt(), output)
Expand All @@ -56,7 +57,7 @@ var _ = Describe("FileUploader", func() {
client := fakeSlmpClientBuilder.GetMtaFiles([]*models.FileMetadata{&testutil.SimpleFile}, nil).Build()
var uploadedFiles []*models.FileMetadata
output := oc.CaptureOutput(func() {
fileUploader = commands.NewFileUploader([]string{}, client)
fileUploader = commands.NewFileUploader([]string{}, client, configuration.DefaultChunkSizeInMB)
uploadedFiles, status = fileUploader.UploadFiles()
})
ex.ExpectSuccess(status.ToInt(), output)
Expand All @@ -72,7 +73,7 @@ var _ = Describe("FileUploader", func() {
UploadMtaFile(*testFile, testutil.GetFile(*testFile, testFileDigest), nil).Build()
var uploadedFiles []*models.FileMetadata
output := oc.CaptureOutput(func() {
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client)
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client, configuration.DefaultChunkSizeInMB)
uploadedFiles, status = fileUploader.UploadFiles()
})
Expect(len(uploadedFiles)).To(Equal(1))
Expand All @@ -94,7 +95,7 @@ var _ = Describe("FileUploader", func() {
UploadMtaFile(*testFile, testutil.GetFile(*testFile, testFileDigest), nil).Build()
var uploadedFiles []*models.FileMetadata
output := oc.CaptureOutput(func() {
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client)
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client, configuration.DefaultChunkSizeInMB)
uploadedFiles, status = fileUploader.UploadFiles()
})
ex.ExpectSuccessWithOutput(status.ToInt(), output, []string{
Expand All @@ -112,7 +113,7 @@ var _ = Describe("FileUploader", func() {
UploadMtaFile(*testFile, fileMetadata, nil).Build()
var uploadedFiles []*models.FileMetadata
output := oc.CaptureOutput(func() {
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client)
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client, configuration.DefaultChunkSizeInMB)
uploadedFiles, status = fileUploader.UploadFiles()
})
Expect(len(uploadedFiles)).To(Equal(1))
Expand All @@ -134,7 +135,7 @@ var _ = Describe("FileUploader", func() {
UploadMtaFile(*testFile, &models.FileMetadata{}, errors.New("Unexpected error from the backend")).Build()
// var uploadedFiles []*models.FileMetadata
output := oc.CaptureOutput(func() {
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client)
fileUploader = commands.NewFileUploader([]string{testFileAbsolutePath}, client, configuration.DefaultChunkSizeInMB)
_, status = fileUploader.UploadFiles()
})
// Expect(len(uploadedFiles)).To(Equal(1))
Expand Down
3 changes: 2 additions & 1 deletion commands/mta_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models"
mtafake "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient/fakes"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/testutil"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
util_fakes "github.com/cloudfoundry-incubator/multiapps-cli-plugin/util/fakes"
Expand Down Expand Up @@ -64,7 +65,7 @@ var _ = Describe("MtaCommand", func() {
testTokenFactory := commands.NewTestTokenFactory(cliConnection)
deployServiceURLCalculator := util_fakes.NewDeployServiceURLFakeCalculator("deploy-service.test.ondemand.com")

command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, clientFactory, testTokenFactory, deployServiceURLCalculator)
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, clientFactory, testTokenFactory, deployServiceURLCalculator, configuration.NewSnapshot())
})

// wrong arguments - error
Expand Down
3 changes: 2 additions & 1 deletion commands/mta_operations_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models"
mtafake "github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient/fakes"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/configuration"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/testutil"
"github.com/cloudfoundry-incubator/multiapps-cli-plugin/ui"
util_fakes "github.com/cloudfoundry-incubator/multiapps-cli-plugin/util/fakes"
Expand Down Expand Up @@ -53,7 +54,7 @@ var _ = Describe("MtaOperationsCommand", func() {
command = &commands.MtaOperationsCommand{}
testTokenFactory := commands.NewTestTokenFactory(cliConnection)
deployServiceURLCalculator := util_fakes.NewDeployServiceURLFakeCalculator("deploy-service.test.ondemand.com")
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, clientFactory, testTokenFactory, deployServiceURLCalculator)
command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200, nil), nil, clientFactory, testTokenFactory, deployServiceURLCalculator, configuration.NewSnapshot())
})

Context("with an unknown flag", func() {
Expand Down
Loading

0 comments on commit ae17597

Please sign in to comment.