[GitHub] mrutkows commented on a change in pull request #298: Fix Parameter Resolution (both schema) & documentation for the triggerrule use case

[gi...@git.apache.org]

mrutkows commented on a change in pull request #298: Fix Parameter Resolution (both schema) & documentation for the triggerrule use case
URL: https://github.com/apache/incubator-openwhisk-wskdeploy/pull/298#discussion_r132932140
 
 

 ##########
 File path: parsers/manifest_parser.go
 ##########
 @@ -403,19 +425,151 @@ func (action *Action) ComposeWskAction(manipath string) (*whisk.Action, error) {
 	return wskaction, err
 }
 
-// Resolve parameter input
-func ResolveParameter(param *Parameter) interface{} {
-	value := utils.GetEnvVar(param.Value)
+
+// TODO(): Support other valid Package Manifest types
+// TODO(): i.e., json (valid), timestamp, version, string256, string64, string16
+// TODO(): Support JSON schema validation for type: json
+// TODO(): Support OpenAPI schema validation
+
+var validParameterNameMap = map[string]string{
+	"string": "string",
+	"int": "integer",
+	"float": "float",
+	"bool": "boolean",
+	"int8": "integer",
+	"int16": "integer",
+	"int32": "integer",
+	"int64": "integer",
+	"float32": "float",
+	"float64": "float",
+}
+
+
+var typeDefaultValueMap = map[string]interface{} {
+	"string": "",
+	"integer": 0,
+	"float": 0.0,
+	"boolean": false,
+	// TODO() Support these types + their validation
+	// timestamp
+	// null
+	// version
+	// string256
+	// string64
+	// string16
+	// json
+	// scalar-unit
+	// schema
+	// object
+}
+
+func isValidParameterType(typeName string) bool {
+	_, isValid := typeDefaultValueMap[typeName]
+	return isValid
+}
+
+// TODO(): throw errors
+func getTypeDefaultValue(typeName string) interface{} {
+
+	if val, ok := typeDefaultValueMap[typeName]; ok {
+		return val
+	} else {
+		// TODO() throw an error "type not found"
+	}
+        return nil
+}
+
+func ResolveParamTypeFromValue(value interface{}) (string, error) {
+        // Note: string is the default type if not specified.
+	var paramType string = "string"
+	var err error = nil
+
+	if value != nil {
+		actualType := reflect.TypeOf(value).Kind().String()
+
+		// See if the actual type of the value is valid
+		if normalizedTypeName, found := validParameterNameMap[actualType]; found {
+			// use the full spec. name
+			paramType = normalizedTypeName
+
+		} else {
+			// raise an error if param is not a known type
+			err = utils.NewParserErr("",-1, "Parameter value is not a known type. [" + actualType + "]")
+		}
+	} else {
+
+		// TODO: The value may be supplied later, we need to support non-fatal warnings
+		// raise an error if param is nil
+		//err = utils.NewParserErr("",-1,"Paramter value is nil.")
+	}
+	return paramType, err
+}
+
+// Resolve input parameter (i.e., type, value, default)
+// Note: parameter values may set later (overriddNen) by an (optional) Deployment file
+func ResolveParameter(paramName string, param *Parameter) (interface{}, error) {
+
+	var errorParser error
+	var tempType string
+	// default parameter value to empty string
+	var value interface{} = ""
+
+	// Trace Parameter struct before any resolution
+	//dumpParameter(paramName, param, "BEFORE")
+
+	// Parameters can be single OR multi-line declarations which must be processed/validated differently
+	if !param.multiline {
+		// we have a single-line parameter declaration
+		// We need to identify parameter Type here for later validation
+		param.Type, errorParser = ResolveParamTypeFromValue(param.Value)
+
+	} else {
+		// we have a multi-line parameter declaration
+
+		// if we do not have a value, but have a default, use it for the value
+                if param.Value == nil && param.Default !=nil {
+			param.Value = param.Default
+		}
+
+		// if we also have a type at this point, verify value (and/or default) matches type, if not error
+		// Note: if either the value or default is in conflict with the type then this is an error
+		tempType, errorParser = ResolveParamTypeFromValue(param.Value)
 
 Review comment:
   Can we do this on a separate issue?  I would like to get this merged for the tests included thus far.  If we try to address all the TODOs I added regarding error/warning handling this issue will get too large IMO. @pritidesai 
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services