Repository: brooklyn-client
Updated Branches:
  refs/heads/master 3dc6921fc -> 7ec86f494


Fix handling of global flags in scope definition

Previously the presence of global flags in a command like
"br --flag app a ent b sensor c" caused the CLI to run the command for
app, not sensor.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-client/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-client/commit/a436dc0f
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-client/tree/a436dc0f
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-client/diff/a436dc0f

Branch: refs/heads/master
Commit: a436dc0f2541a1d52c35fef00fe40efb4aec1968
Parents: be80516
Author: Sam Corbett <[email protected]>
Authored: Thu Jul 27 14:58:11 2017 +0100
Committer: Sam Corbett <[email protected]>
Committed: Thu Jul 27 14:58:11 2017 +0100

----------------------------------------------------------------------
 cli/api/catalog/catalog.go |  2 +-
 cli/commands/activity.go   |  2 +-
 cli/scope/scope.go         | 41 ++++++++----------
 cli/scope/scope_test.go    | 92 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 111 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-client/blob/a436dc0f/cli/api/catalog/catalog.go
----------------------------------------------------------------------
diff --git a/cli/api/catalog/catalog.go b/cli/api/catalog/catalog.go
index d8132ca..13944b4 100644
--- a/cli/api/catalog/catalog.go
+++ b/cli/api/catalog/catalog.go
@@ -24,7 +24,7 @@ import (
        "github.com/apache/brooklyn-client/cli/models"
        "github.com/apache/brooklyn-client/cli/net"
        "net/url"
-        "path/filepath"
+       "path/filepath"
        "errors"
        "os"
        "strings"

http://git-wip-us.apache.org/repos/asf/brooklyn-client/blob/a436dc0f/cli/commands/activity.go
----------------------------------------------------------------------
diff --git a/cli/commands/activity.go b/cli/commands/activity.go
index 1d82dac..a98cbc7 100644
--- a/cli/commands/activity.go
+++ b/cli/commands/activity.go
@@ -50,7 +50,7 @@ func (cmd *Activity) Metadata() 
command_metadata.CommandMetadata {
                Name:        "activity",
                Aliases:     []string{"activities", "act", "acts"},
                Description: "Show the activity for an application / entity",
-               Usage:       "BROOKLYN_NAME SCOPE activity [ ACTIVITYID]",
+               Usage:       "BROOKLYN_NAME SCOPE activity [ACTIVITYID]",
                Flags: []cli.Flag{
                        cli.StringSliceFlag{
                                Name:  "children, c",

http://git-wip-us.apache.org/repos/asf/brooklyn-client/blob/a436dc0f/cli/scope/scope.go
----------------------------------------------------------------------
diff --git a/cli/scope/scope.go b/cli/scope/scope.go
index 9332c4a..dbff917 100644
--- a/cli/scope/scope.go
+++ b/cli/scope/scope.go
@@ -89,36 +89,38 @@ var scopeSpecifier = map[string]func(scope *Scope, id 
string){
 // the function will return ([]string{"br", "doSomething"}, 
Scope{Application:1, Entity:2})
 func ScopeArguments(args []string) ([]string, Scope) {
        scope := Scope{}
-
        if len(args) < 2 {
                return args, scope
        }
-
-       command := args[0]
-       args = args[1:]
-
        args = defineScope(args, &scope)
-
-       args = prepend(command, args)
-
        return args, scope
 }
 
 func defineScope(args []string, scope *Scope) []string {
+       // args is: name [global flags] [scope and id pair] command [flags] 
[arguments]
+       // Skip name and global flags
+       idx := 1
+       for isFlag(args[idx]) {
+               idx += 1
+       }
+       newArgs := args[:idx]
+       args = args[idx:]
 
-       allScopesFound := false
-       for !allScopesFound && len(args) > 2 && args[1][0] != '-' {
-               if setAppropriateScope, nameOfAScope := 
scopeSpecifier[args[0]]; nameOfAScope {
-                       setAppropriateScope(scope, args[1])
+       for len(args) > 2 && !isFlag(args[1]) {
+               if scopeFn, nameOfAScope := scopeSpecifier[args[0]]; 
nameOfAScope {
+                       scopeFn(scope, args[1])
                        args = args[2:]
                } else {
-                       allScopesFound = true
+                       break
                }
        }
-
        setDefaultEntityIfRequired(scope)
+       return append(newArgs, args...)
+}
 
-       return args
+// true if the first character of arg is -
+func isFlag(arg string) bool {
+       return arg[0] == '-'
 }
 
 func setDefaultEntityIfRequired(scope *Scope) {
@@ -126,12 +128,3 @@ func setDefaultEntityIfRequired(scope *Scope) {
                scope.Entity = scope.Application
        }
 }
-
-func prepend(v string, args []string) []string {
-       result := make([]string, len(args)+1)
-       result[0] = v
-       for i, a := range args {
-               result[i+1] = a
-       }
-       return result
-}

http://git-wip-us.apache.org/repos/asf/brooklyn-client/blob/a436dc0f/cli/scope/scope_test.go
----------------------------------------------------------------------
diff --git a/cli/scope/scope_test.go b/cli/scope/scope_test.go
new file mode 100644
index 0000000..b18e207
--- /dev/null
+++ b/cli/scope/scope_test.go
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package scope
+
+import (
+       "testing"
+)
+
+type testCase struct{
+       Args []string
+       ExpectedArgs []string
+       ExpectedScope Scope
+}
+
+func TestScope(t *testing.T) {
+       testCases := []testCase {
+               {
+                       Args: []string{"br", "application"},
+                       ExpectedArgs: []string{"br", "application"},
+                       ExpectedScope: Scope{},
+               }, {
+                       Args: []string{"br", "application", "appid", "entity"},
+                       ExpectedArgs: []string{"br", "entity"},
+                       ExpectedScope: Scope{Application: "appid", Entity: 
"appid"},
+               }, {
+                       Args: []string{"br", "application", "appid", "entity", 
"entity"},
+                       ExpectedArgs: []string{"br", "entity", "entity"},
+                       // ScopeArguments sets Entity incorrectly when the 
command is entity.
+                       ExpectedScope: Scope{Application: "appid", Entity: 
"appid"},
+               }, {
+                       Args: []string{"br", "application", "appid", "entity", 
"entity", "sensor"},
+                       ExpectedArgs: []string{"br", "sensor"},
+                       ExpectedScope: Scope{Application: "appid", Entity: 
"entity"},
+               }, {
+                       Args: []string{"br", "--verbose", "application", 
"appid", "entity", "entityId", "sensor"},
+                       ExpectedArgs: []string{"br", "--verbose", "sensor"},
+                       ExpectedScope: Scope{Application: "appid", Entity: 
"entityId"},
+               }, {
+                       Args: []string{"br", "--verbose", "application", 
"appid", "entity", "entityId", "sensor", "http.port"},
+                       ExpectedArgs: []string{"br", "--verbose", "sensor", 
"http.port"},
+                       ExpectedScope: Scope{Application: "appid", Entity: 
"entityId"},
+               }, {
+                       Args: []string{"br", "--verbose", "a", "appid", "e", 
"entityId", "v", "--children", "activityId"},
+                       ExpectedArgs: []string{"br", "--verbose", "v", 
"--children", "activityId"},
+                       ExpectedScope: Scope{Application: "appid", Entity: 
"entityId"},
+               },
+       }
+       for _, elem := range testCases {
+               argsOut, scope := ScopeArguments(elem.Args)
+               assertArgs(t, argsOut, elem.ExpectedArgs)
+               assertScope(t, scope, elem.ExpectedScope)
+       }
+}
+
+func assertArgs(t *testing.T, actual []string, expected []string) {
+       if len(actual) != len(expected) {
+               t.Errorf("%q != %q", actual, expected)
+               t.FailNow()
+       }
+       for idx, act := range actual {
+               exp := expected[idx]
+               if act != exp {
+                       t.Errorf("mismatch at index %d: %q != %q", idx, actual, 
expected)
+               }
+       }
+}
+
+func assertScope(t *testing.T, actual Scope, expected Scope) {
+       if actual.Application != expected.Application ||
+               actual.Activity != expected.Activity ||
+               actual.Config != expected.Config ||
+               actual.Effector != expected.Effector ||
+               actual.Entity != expected.Entity {
+               t.Errorf("%v != %v", actual, expected)
+       }
+}

Reply via email to