Skip to content

Commit 42307de

Browse files
rabbahVincent
rabbah
authored and
Vincent
committed
Refactor trigger create using new utility methods to factor out printing of the feed action result (#315)
* Refactoring to remove unnecessary clone and own code. This commit update trigger creation with a feed to use the new invoke methods to both invoke and display the response. It will next be possible to remove the activation result on success. * Fix Go and Scala tests which accepted optional payload. * Factor out an error message identifier and edit its text.
1 parent c07b59d commit 42307de

File tree

5 files changed

+132
-106
lines changed

5 files changed

+132
-106
lines changed

‎commands/messages.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package commands
19+
20+
/**
21+
* Mapping from identifiers to message ids in wski18n resources.
22+
*
23+
* NOTE: this list is not complete as message will be moved incrementally.
24+
*/
25+
const (
26+
FEED_CONFIGURATION_FAILURE = "FEED_CONFIGURATION_FAILURE"
27+
)

‎commands/trigger.go‎

Lines changed: 97 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,58 @@ var triggerCmd = &cobra.Command{
4242
Short: wski18n.T("work with triggers"),
4343
}
4444

45+
/**
46+
* Constructs the parameters to pass to the feed, consisting of the event type, trigger name, and subject key.
47+
*
48+
* NOTE: this function will exit in case of a processing error since it indicates a problem parsing parameters.
49+
*
50+
* @return the feed name and parameters if a feed is specified.
51+
*/
52+
func feedParameters(feed string, lifecycle string, trigger *QualifiedName, authKey string) (*QualifiedName, []string) {
53+
if feed == "" {
54+
return nil, make([]string, 0)
55+
}
56+
57+
whisk.Debug(whisk.DbgInfo, "Trigger has a feed\n")
58+
59+
var params []string
60+
name := fmt.Sprintf("/%s/%s", trigger.GetNamespace(), trigger.GetEntityName())
61+
params = append(params, getFormattedJSON(FEED_LIFECYCLE_EVENT, lifecycle))
62+
params = append(params, getFormattedJSON(FEED_TRIGGER_NAME, name))
63+
params = append(params, getFormattedJSON(FEED_AUTH_KEY, authKey))
64+
65+
feedQualifiedName, err := NewQualifiedName(feed)
66+
if err != nil {
67+
ExitOnError(NewQualifiedNameError(feed, err))
68+
}
69+
70+
return feedQualifiedName, params
71+
}
72+
73+
/**
74+
* Create or update a trigger.
75+
*
76+
* NOTE: this function will exit in case of a processing error since it indicates a problem parsing parameters.
77+
*/
78+
func createOrUpdate(fqn *QualifiedName, trigger *whisk.Trigger, overwrite bool) {
79+
// TODO get rid of these global modifiers
80+
Client.Namespace = fqn.GetNamespace()
81+
_, _, err := Client.Triggers.Insert(trigger, overwrite)
82+
83+
if err != nil {
84+
whisk.Debug(whisk.DbgError, "Client.Triggers.Insert(%+v, %s) failed: %s\n", trigger, overwrite, err)
85+
var errStr string
86+
if !overwrite {
87+
errStr = wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
88+
map[string]interface{}{"name": trigger.Name, "err": err})
89+
} else {
90+
errStr = wski18n.T("Unable to update trigger '{{.name}}': {{.err}}",
91+
map[string]interface{}{"name": trigger.Name, "err": err})
92+
}
93+
ExitOnError(whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE))
94+
}
95+
}
96+
4597
var triggerFireCmd = &cobra.Command{
4698
Use: "fire TRIGGER_NAME [PAYLOAD]",
4799
Short: wski18n.T("fire trigger event"),
@@ -50,38 +102,21 @@ var triggerFireCmd = &cobra.Command{
50102
PreRunE: SetupClientConfig,
51103
RunE: func(cmd *cobra.Command, args []string) error {
52104
var err error
53-
var parameters interface{}
54105
var qualifiedName = new(QualifiedName)
55106

56-
if whiskErr := CheckArgs(args, 1, 2, "Trigger fire",
57-
wski18n.T("A trigger name is required. A payload is optional.")); whiskErr != nil {
107+
if whiskErr := CheckArgs(args, 1, 1, "Trigger fire",
108+
wski18n.T("A trigger name is required.")); whiskErr != nil {
58109
return whiskErr
59110
}
60111

61112
if qualifiedName, err = NewQualifiedName(args[0]); err != nil {
62113
return NewQualifiedNameError(args[0], err)
63114
}
64115

65-
Client.Namespace = qualifiedName.GetNamespace()
66-
67-
// Add payload to parameters
68-
if len(args) == 2 {
69-
Flags.common.param = append(Flags.common.param, getFormattedJSON("payload", args[1]))
70-
Flags.common.param = append(Flags.common.param, Flags.common.param...)
71-
}
72-
73-
if len(Flags.common.param) > 0 {
74-
parameters, err = getJSONFromStrings(Flags.common.param, false)
75-
if err != nil {
76-
whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, false) failed: %s\n", Flags.common.param, err)
77-
errStr := wski18n.T("Invalid parameter argument '{{.param}}': {{.err}}",
78-
map[string]interface{}{"param": fmt.Sprintf("%#v", Flags.common.param), "err": err})
79-
werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL,
80-
whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
81-
return werr
82-
}
83-
}
116+
parameters := getParameters(Flags.common.param, false, false)
84117

118+
// TODO get rid of these global modifiers
119+
Client.Namespace = qualifiedName.GetNamespace()
85120
trigResp, _, err := Client.Triggers.Fire(qualifiedName.GetEntityName(), parameters)
86121
if err != nil {
87122
whisk.Debug(whisk.DbgError, "Client.Triggers.Fire(%s, %#v) failed: %s\n", qualifiedName.GetEntityName(), parameters, err)
@@ -110,103 +145,73 @@ var triggerCreateCmd = &cobra.Command{
110145
SilenceErrors: true,
111146
PreRunE: SetupClientConfig,
112147
RunE: func(cmd *cobra.Command, args []string) error {
113-
var err error
114-
var annotations interface{}
115-
var feedArgPassed bool = (Flags.common.feed != "")
116-
var qualifiedName = new(QualifiedName)
117-
118148
if whiskErr := CheckArgs(args, 1, 1, "Trigger create",
119149
wski18n.T("A trigger name is required.")); whiskErr != nil {
120150
return whiskErr
121151
}
122152

123-
if qualifiedName, err = NewQualifiedName(args[0]); err != nil {
124-
return NewQualifiedNameError(args[0], err)
125-
}
126-
127-
Client.Namespace = qualifiedName.GetNamespace()
128-
129-
var fullTriggerName string
130-
var fullFeedName string
131-
var feedQualifiedName = new(QualifiedName)
132-
if feedArgPassed {
133-
whisk.Debug(whisk.DbgInfo, "Trigger has a feed\n")
134-
135-
if feedQualifiedName, err = NewQualifiedName(Flags.common.feed); err != nil {
136-
return NewQualifiedNameError(Flags.common.feed, err)
137-
}
138-
139-
fullFeedName = fmt.Sprintf("/%s/%s", feedQualifiedName.GetNamespace(), feedQualifiedName.GetEntityName())
140-
fullTriggerName = fmt.Sprintf("/%s/%s", qualifiedName.GetNamespace(), qualifiedName.GetEntityName())
141-
Flags.common.param = append(Flags.common.param, getFormattedJSON(FEED_LIFECYCLE_EVENT, FEED_CREATE))
142-
Flags.common.param = append(Flags.common.param, getFormattedJSON(FEED_TRIGGER_NAME, fullTriggerName))
143-
Flags.common.param = append(Flags.common.param, getFormattedJSON(FEED_AUTH_KEY, Client.Config.AuthToken))
144-
}
145-
146-
// Convert the trigger's list of default parameters from a string into []KeyValue
147-
// The 1 or more --param arguments have all been combined into a single []string
148-
// e.g. --p arg1,arg2 --p arg3,arg4 -> [arg1, arg2, arg3, arg4]
149-
whisk.Debug(whisk.DbgInfo, "Parsing parameters: %#v\n", Flags.common.param)
150-
parameters, err := getJSONFromStrings(Flags.common.param, !feedArgPassed)
151-
153+
triggerName, err := NewQualifiedName(args[0])
152154
if err != nil {
153-
whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.param, err)
154-
errStr := wski18n.T("Invalid parameter argument '{{.param}}': {{.err}}",
155-
map[string]interface{}{"param": fmt.Sprintf("%#v", Flags.common.param), "err": err})
156-
werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
157-
return werr
155+
return NewQualifiedNameError(args[0], err)
158156
}
159157

160-
// Add feed to annotations
161-
if feedArgPassed {
162-
Flags.common.annotation = append(Flags.common.annotation, getFormattedJSON("feed", Flags.common.feed))
163-
}
158+
paramArray := Flags.common.param
159+
annotationArray := Flags.common.annotation
160+
feedParam := Flags.common.feed
161+
authToken := Client.Config.AuthToken
164162

165-
whisk.Debug(whisk.DbgInfo, "Parsing annotations: %#v\n", Flags.common.annotation)
166-
annotations, err = getJSONFromStrings(Flags.common.annotation, true)
163+
// if a feed is specified, create additional parameters which must be passed to the feed
164+
feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
165+
// the feed receives all the paramaters that are specified on the command line so we merge
166+
// the feed lifecycle parameters with the command line ones
167+
parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
167168

168-
if err != nil {
169-
whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.annotation, err)
170-
errStr := wski18n.T("Invalid annotation argument '{{.annotation}}': {{.err}}",
171-
map[string]interface{}{"annotation": fmt.Sprintf("%#v", Flags.common.annotation), "err": err})
172-
werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
173-
return werr
169+
// if a feed is specified, add feed annotation the annotations declared on the command line
170+
// TODO: add test to ensure that generated annotation has precedence
171+
if feedName != nil {
172+
annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
174173
}
174+
annotations := getParameters(annotationArray, true, true)
175175

176176
trigger := &whisk.Trigger{
177-
Name: qualifiedName.GetEntityName(),
177+
Name: triggerName.GetEntityName(),
178178
Annotations: annotations.(whisk.KeyValueArr),
179179
}
180180

181-
if !feedArgPassed {
181+
if feedName == nil {
182+
// parameters are only attached to the trigger in there is no feed, otherwise
183+
// parameters are passed to the feed instead
182184
trigger.Parameters = parameters.(whisk.KeyValueArr)
183185
}
184186

185-
_, _, err = Client.Triggers.Insert(trigger, false)
186-
if err != nil {
187-
whisk.Debug(whisk.DbgError, "Client.Triggers.Insert(%+v,false) failed: %s\n", trigger, err)
188-
errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
189-
map[string]interface{}{"name": trigger.Name, "err": err})
190-
werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
191-
return werr
192-
}
187+
createOrUpdate(triggerName, trigger, false)
193188

194189
// Invoke the specified feed action to configure the trigger feed
195-
if feedArgPassed {
196-
err := configureFeed(trigger.Name, fullFeedName, getParameters(Flags.common.param, false, false))
190+
if feedName != nil {
191+
res, err := invokeAction(*feedName, parameters, true, false)
197192
if err != nil {
198-
whisk.Debug(whisk.DbgError, "configureFeed(%s, %s) failed: %s\n", trigger.Name, Flags.common.feed,
199-
err)
193+
whisk.Debug(whisk.DbgError, "Failed configuring feed '%s' failed: %s\n", feedName.GetFullQualifiedName(), err)
194+
195+
// TODO: should we do this at all? Keeping for now.
196+
printFailedBlockingInvocationResponse(*feedName, false, res, err)
197+
198+
reason := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": feedName.GetFullQualifiedName(), "err": err})
200199
errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
201-
map[string]interface{}{"name": trigger.Name, "err": err})
200+
map[string]interface{}{"name": trigger.Name, "err": reason})
202201
werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
203202

204203
// Delete trigger that was created for this feed
205-
delerr := deleteTrigger(args[0])
206-
if delerr != nil {
207-
whisk.Debug(whisk.DbgWarn, "Ignoring deleteTrigger(%s) failure: %s\n", args[0], delerr)
204+
err = deleteTrigger(triggerName.GetEntityName())
205+
if err != nil {
206+
whisk.Debug(whisk.DbgWarn, "Ignoring deleteTrigger(%s) failure: %s\n", triggerName.GetEntityName(), err)
208207
}
208+
209209
return werr
210+
} else {
211+
whisk.Debug(whisk.DbgInfo, "Successfully configured trigger feed via feed action '%s'\n", feedName)
212+
213+
// preserve existing behavior where output of feed activation is emitted to console
214+
printInvocationMsg(*feedName, true, true, res, color.Output)
210215
}
211216
}
212217

@@ -351,7 +356,6 @@ var triggerGetCmd = &cobra.Command{
351356
}
352357

353358
Client.Namespace = qualifiedName.GetNamespace()
354-
355359
retTrigger, _, err := Client.Triggers.Get(qualifiedName.GetEntityName())
356360
if err != nil {
357361
whisk.Debug(whisk.DbgError, "Client.Triggers.Get(%s) failed: %s\n", qualifiedName.GetEntityName(), err)
@@ -449,7 +453,6 @@ var triggerDeleteCmd = &cobra.Command{
449453
Flags.common.param = origParams
450454
Client.Namespace = qualifiedName.GetNamespace()
451455
}
452-
453456
}
454457

455458
retTrigger, _, err = Client.Triggers.Delete(qualifiedName.GetEntityName())
@@ -528,8 +531,7 @@ func configureFeed(triggerName string, feedName string, parameters interface{})
528531

529532
if err != nil {
530533
whisk.Debug(whisk.DbgError, "Invoke of action '%s' failed: %s\n", feedName, err)
531-
errStr := wski18n.T("Unable to invoke trigger '{{.trigname}}' feed action '{{.feedname}}'; feed is not configured: {{.err}}",
532-
map[string]interface{}{"trigname": triggerName, "feedname": feedName, "err": err})
534+
errStr := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": feedName, "trigname": triggerName, "err": err})
533535
err = whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
534536
} else {
535537
whisk.Debug(whisk.DbgInfo, "Successfully configured trigger feed via feed action '%s'\n", feedName)
@@ -580,5 +582,4 @@ func init() {
580582
triggerDeleteCmd,
581583
triggerListCmd,
582584
)
583-
584585
}

‎tests/src/integration/integration_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ var ruleTriggerActionReqMsg = "A rule, trigger and action name are required."
4242
var activationIdReq = "An activation ID is required."
4343
var triggerNameReqMsg = "A trigger name is required."
4444
var optNamespaceMsg = "An optional namespace is the only valid argument."
45-
var optPayloadMsg = "A payload is optional."
4645
var noArgsReqMsg = "No arguments are required."
4746
var invalidArg = "invalidArg"
4847
var apiCreateReqMsg = "Specify a swagger file or specify an API base path with an API path, an API verb, and an action name."
@@ -282,11 +281,11 @@ func initInvalidArgs() {
282281

283282
{
284283
Cmd: []string{"trigger", "fire"},
285-
Err: tooFewArgsMsg + " " + triggerNameReqMsg + " " + optPayloadMsg,
284+
Err: tooFewArgsMsg + " " + triggerNameReqMsg,
286285
},
287286
{
288-
Cmd: []string{"trigger", "fire", "triggerName", "triggerPayload", invalidArg},
289-
Err: tooManyArgsMsg + invalidArg + ". " + triggerNameReqMsg + " " + optPayloadMsg,
287+
Cmd: []string{"trigger", "fire", "triggerName", invalidArg},
288+
Err: tooManyArgsMsg + invalidArg + ". " + triggerNameReqMsg,
290289
},
291290
{
292291
Cmd: []string{"trigger", "create"},

‎tests/src/test/scala/whisk/core/cli/test/WskCliBasicUsageTests.scala‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2257,7 +2257,6 @@ class WskCliBasicUsageTests extends TestHelpers with WskTestHelpers {
22572257
val activationIdReq = "An activation ID is required."
22582258
val triggerNameReqMsg = "A trigger name is required."
22592259
val optNamespaceMsg = "An optional namespace is the only valid argument."
2260-
val optPayloadMsg = "A payload is optional."
22612260
val noArgsReqMsg = "No arguments are required."
22622261
val invalidArg = "invalidArg"
22632262
val apiCreateReqMsg =
@@ -2389,9 +2388,9 @@ class WskCliBasicUsageTests extends TestHelpers with WskTestHelpers {
23892388
s"${tooManyArgsMsg}${invalidArg}. ${optNamespaceMsg}"),
23902389
(Seq("rule", "list", invalidArg), entityNameMsg),
23912390
(Seq("trigger", "fire"),
2392-
s"${tooFewArgsMsg} ${triggerNameReqMsg} ${optPayloadMsg}"),
2393-
(Seq("trigger", "fire", "triggerName", "triggerPayload", invalidArg),
2394-
s"${tooManyArgsMsg}${invalidArg}. ${triggerNameReqMsg} ${optPayloadMsg}"),
2391+
s"${tooFewArgsMsg} ${triggerNameReqMsg}"),
2392+
(Seq("trigger", "fire", "triggerName", invalidArg),
2393+
s"${tooManyArgsMsg}${invalidArg}. ${triggerNameReqMsg}"),
23952394
(Seq("trigger", "create"), s"${tooFewArgsMsg} ${triggerNameReqMsg}"),
23962395
(Seq("trigger", "create", "triggerName", invalidArg),
23972396
s"${tooManyArgsMsg}${invalidArg}."),

‎wski18n/resources/en_US.all.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,8 @@
687687
"translation": "Unable to obtain the list of triggers for namespace '{{.name}}': {{.err}}"
688688
},
689689
{
690-
"id": "Unable to invoke trigger '{{.trigname}}' feed action '{{.feedname}}'; feed is not configured: {{.err}}",
691-
"translation": "Unable to invoke trigger '{{.trigname}}' feed action '{{.feedname}}'; feed is not configured: {{.err}}"
690+
"id": "FEED_CONFIGURATION_FAILURE",
691+
"translation": "Unable to configure feed '{{.feedname}}': {{.err}}"
692692
},
693693
{
694694
"note-to-translators": "DO NOT TRANSLATE THE 'yes' AND 'no'. THOSE ARE FIXED CLI ARGUMENT VALUES",

0 commit comments

Comments
 (0)