Merge pull request #16 from intersystems/internal-updates

Upstream updates for 1.1.0 release

Author: isc-tleavitt

CHANGELOG.md

@@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased - 1.1.0+snapshot]
+## [1.1.0] - 2022-08-08
 
 ### Added 
 - APPS-12985: Support removal of certain endpoints at the dispatch class and resource level
@@ -15,22 +15,20 @@ via new `Supports()` method that can be overridden at REST handler and resource
 appropriate class members are overridden in subclasses.
 - APPS-12782: Support for fallback/default mimetype/representation of a resource when a 
 regular expression of either `*/*` or `application/*` is used in an Accept header.
+- APPS-13359: Add appropriate error handling for unsupported return data types for custom actions.
+- APPS-13361: Add support for return type of literals i.e. datatype classes for actions.
+- APPS-13650: Add support for %CSP.Stream for return type of actions.
 
 ### Changed
 - APPS-13152: Locked down methods as final in classes part of public API.
+- APPS-13361: Remove constraint of JSON for action return types in handlers (keep it for content for now).
 
 ### Fixed
 - APPS-13327: Fix a small issue in `$$$OperationAction` macro where lack of 
 parentheses could cause invalid equality checks against an action name.
-
-### Security
--
-
-### Removed
-- 
-
-### Deprecated
--
+- APPS-13388: Swap `write` for `do` when using `%ToJSON()` to write to the current 
+device to avoid `<MAXLEN>` errors.
+- APPS-13698: Prevent spurious compilation errors validating default representations
 
 ## [1.0.1] - 2022-05-25
 - Last released version before CHANGELOG existed.

cls/_pkg/isc/rest/handler.cls

@@ -211,7 +211,7 @@ ClassMethod GetDocumentationBuildStatus() As %Status [ Final ]
 	} Catch {
 		Set buildParameters.userContext = $Get($$$DocParameterBuildUserGbl,"")
 	}
-	Write status.%ToJSON()
+	Do status.%ToJSON()
 	Return $$$OK
 }
 
@@ -245,7 +245,7 @@ ClassMethod BuildDocumentationEndpoint() As %Status [ Final ]
 		Do ..ReportHttpStatusCode(..#HTTP423LOCKED)
 		Set %response.ContentType = "application/json"
 		Set status = {"statusUrl":"/build-documentation","statusMethod":"GET","buildStarted":false}
-		Write status.%ToJSON()
+		Do status.%ToJSON()
 		Return $$$OK
 	}
 	Set $$$DocLocksBuildGbl = 1
@@ -257,7 +257,7 @@ ClassMethod BuildDocumentationEndpoint() As %Status [ Final ]
 	Set %response.ContentType = "application/json"
 	Set status = {"statusUrl":"/build-documentation","statusMethod":"GET"} 
 	Do status.%Set("buildStarted", $Test, "boolean")
-	Write status.%ToJSON()
+	Do status.%ToJSON()
 	Return $$$OK
 }
 
@@ -377,15 +377,15 @@ ClassMethod Supports(pEndpoint As %String, pHTTPVerb As %String, pRequest As %CS
 /// Check whether a given operation is supported for the resource.
 /// Returns 0 if the operation is not supported and sets the response status
 /// to 404 Not Found. <br />
-ClassMethod CheckSupports(pResourceClass As %Dictionary.CacheClassname, pOperation As %String, pType As %String) As %Boolean [ Final, Internal, Private ]
+ClassMethod CheckSupports(pResourceClass As %Dictionary.Classname, pOperation As %String, pType As %String) As %Boolean [ Final, Internal, Private ]
 {
 	// Wrap with $Get() on %request in case this is called during Open API generation
 	// and %request is not available.
 	Return $ClassMethod(pResourceClass, "Supports", pOperation, pType, $Get(%request))
 }
 
 /// Checks both strategy- and resource-level permissions
-ClassMethod CheckAllPermissions(pResourceClass As %Dictionary.CacheClassname, pID As %String, pOperation As %String, pUserContext As %String, ByRef pURLParams) As %Boolean [ Final ]
+ClassMethod CheckAllPermissions(pResourceClass As %Dictionary.Classname, pID As %String, pOperation As %String, pUserContext As %String, ByRef pURLParams) As %Boolean [ Final ]
 {
 	Set strategy = ..AuthenticationStrategy()
 	Set authorized = 1
@@ -504,7 +504,7 @@ ClassMethod Create(resourceName As %String) As %Status [ Final ]
 
 /// Wrapper around <method>Supports</method> for the <method>Create</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsCreate(pResourceClass As %Dictionary.CacheClassname) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsCreate(pResourceClass As %Dictionary.Classname) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationCreate, $$$TypeOperationClass)
 }
@@ -557,7 +557,7 @@ ClassMethod CollectionQuery(resourceName As %String) As %Status [ Final ]
 
 /// Wrapper around <method>Supports</method> for the <method>CollectionQuery</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsCollectionQuery(pResourceClass As %Dictionary.CacheClassname) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsCollectionQuery(pResourceClass As %Dictionary.Classname) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationQuery, $$$TypeOperationClass)
 }
@@ -593,7 +593,7 @@ ClassMethod Retrieve(resourceName As %String, id As %String) As %Status [ Final
 
 /// Wrapper around <method>Supports</method> for the <method>Retrieve</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsRetrieve(pResourceClass As %Dictionary.CacheClassname) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsRetrieve(pResourceClass As %Dictionary.Classname) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationRead, $$$TypeOperationInstance)
 }
@@ -634,7 +634,7 @@ ClassMethod Construct(resourceName As %String) As %Status [ Final ]
 /// Wrapper around <method>Supports</method> for the <method>Construct</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
 /// $$$OperationRead with no ID, or $$$OperationActionNew, is usable as validation for this special case.
-ClassMethod CheckSupportsConstruct(pResourceClass As %Dictionary.CacheClassname) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsConstruct(pResourceClass As %Dictionary.Classname) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationRead, $$$TypeOperationClass) ||
 ..CheckSupports(pResourceClass, $$$OperationActionNew, $$$TypeOperationClass)
@@ -678,7 +678,7 @@ ClassMethod Update(resourceName As %String, id As %String) As %Status [ Final ]
 
 /// Wrapper around <method>Supports</method> for the <method>Update</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsUpdate(pResourceClass As %Dictionary.CacheClassname) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsUpdate(pResourceClass As %Dictionary.Classname) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationUpdate, $$$TypeOperationInstance)
 }
@@ -718,7 +718,7 @@ ClassMethod Delete(resourceName As %String, id As %String) As %Status [ Final ]
 
 /// Wrapper around <method>Supports</method> for the <method>Delete</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsDelete(pResourceClass As %Dictionary.CacheClassname) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsDelete(pResourceClass As %Dictionary.Classname) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationDelete, $$$TypeOperationInstance)
 }
@@ -765,7 +765,7 @@ ClassMethod DispatchClassAction(resourceName As %String, action As %String) As %
 
 /// Wrapper around <method>Supports</method> for the <method>DispatchClassAction</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsDispatchClassAction(pResourceClass As %Dictionary.CacheClassname, pAction As %String) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsDispatchClassAction(pResourceClass As %Dictionary.Classname, pAction As %String) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationAction(pAction), $$$TypeOperationClass)
 }
@@ -813,12 +813,12 @@ ClassMethod DispatchInstanceAction(resourceName As %String, id As %String, actio
 
 /// Wrapper around <method>Supports</method> for the <method>DispatchInstanceAction</method>
 /// operation. Use this wrapper so it can be invoked from Open API generation.
-ClassMethod CheckSupportsDispatchInstanceAction(pResourceClass As %Dictionary.CacheClassname, pAction As %String) [ CodeMode = expression, Final, Internal ]
+ClassMethod CheckSupportsDispatchInstanceAction(pResourceClass As %Dictionary.Classname, pAction As %String) [ CodeMode = expression, Final, Internal ]
 {
 ..CheckSupports(pResourceClass, $$$OperationAction(pAction), $$$TypeOperationInstance)
 }
 
-ClassMethod FindActionClass(pResource As %String, pAction As %String, pTarget As %String, Output pResourceClass As %Dictionary.CacheClassname) As %Dictionary.CacheClassname [ Final, Private ]
+ClassMethod FindActionClass(pResource As %String, pAction As %String, pTarget As %String, Output pResourceClass As %Dictionary.Classname) As %Dictionary.Classname [ Final, Private ]
 {
 	#dim %response As %CSP.Response
 	#dim %request As %CSP.Request
@@ -827,19 +827,6 @@ ClassMethod FindActionClass(pResource As %String, pAction As %String, pTarget As
 	Set contentType = ..GetMediaTypeFromContentType()
 	If (acceptsList = "") {
 		Set acceptsList = $ListBuild($Char(0))
-	} Else {
-		// Validation
-		Set ptr = 0
-		While $ListNext(acceptsList,ptr,accepts) {
-			If (accepts = "*/*") || (accepts = "application/*") {
-				// TODO: Support this with fallback
-				Continue
-			}
-			If '$Match(accepts,..#AllowedMediaTypeRegex) {
-				Do ..ReportHttpStatusCode(..#HTTP406NOTACCEPTABLE, $$$ERROR($$$GeneralError,"Only JSON-based media types are supported."))
-				Return ""
-			}
-		}
 	}
 
 	Set contentTypeList = $ListBuild(contentType)
@@ -886,19 +873,19 @@ ClassMethod FindActionClass(pResource As %String, pAction As %String, pTarget As
 	Return ""
 }
 
-ClassMethod FindAcceptedClass(pResource As %String) As %Dictionary.CacheClassname [ Final, Private ]
+ClassMethod FindAcceptedClass(pResource As %String) As %Dictionary.Classname [ Final, Private ]
 {
 	#dim %request As %CSP.Request
 	Quit ..FindClass(..GetMediaTypeListFromAcceptHeader(),pResource,..#HTTP406NOTACCEPTABLE,1)
 }
 
-ClassMethod FindContentClass(pResource As %String) As %Dictionary.CacheClassname [ Final, Private ]
+ClassMethod FindContentClass(pResource As %String) As %Dictionary.Classname [ Final, Private ]
 {
 	#dim %request As %CSP.Request
 	Quit ..FindClass($ListBuild(..GetMediaTypeFromContentType()),pResource,..#HTTP415UNSUPPORTEDMEDIATYPE)
 }
 
-ClassMethod FindClass(pMediaTypeList As %Library.List, pResource As %String, pStatusWhenInvalid As %String, pLookForDefault As %Boolean = 0) As %Dictionary.CacheClassname [ Final, Private ]
+ClassMethod FindClass(pMediaTypeList As %Library.List, pResource As %String, pStatusWhenInvalid As %String, pLookForDefault As %Boolean = 0) As %Dictionary.Classname [ Final, Private ]
 {
 	Set pointer = 0
 	Set resourceClass = ""
@@ -1088,7 +1075,7 @@ ClassMethod outputStatus(pSC As %Status) As %Status [ Internal ]
             Set tSC = ..StatusToJSON(pSC, .tJSON)
             If $$$ISERR(tSC) Quit
             // Write the JSON to the output device
-           	Write tJSON.%ToJSON()
+           	Do tJSON.%ToJSON()
         } Else {
             // Set plain text
             Set %response.ContentType = ..#CONTENTTYPETEXT

cls/_pkg/isc/rest/model/action/t/action.cls

@@ -43,11 +43,13 @@ Method %OnValidateObject() As %Status [ Private, ServerOnly = 1 ]
 	}
 
 	If (..query '= "") {
-		If (..method = "POST") {
+		// query
+		If (..method '= "GET") {
 			Set msg = $$$FormatText("Action %1: must use GET method with a query.",..name)
 			Return $$$ERROR($$$GeneralError,msg)
 		}
 	} Else {
+		// call
 		If (..modelClass '= "") {
 			Set msg = $$$FormatText("Action %1: modelClass may only be defined for query actions",..name)
 			Return $$$ERROR($$$GeneralError,msg)
@@ -341,52 +343,113 @@ Method Generate(pSourceClass As %String, Output pCodeArray, Output pExpectedCont
 		}
 	}
 
+	Set resultVariableName = "result"
 	If (..query '= "") {
-		Set methodCall = "set result = ##class(%pkg.isc.rest.model.queryResult).FromClassQuery("_$$$QUOTE(tResultClass)_","_$$$QUOTE(class)_","_$$$QUOTE(method)_",args...)"
+		Set methodCall = "Set "_resultVariableName_" = ##class(%pkg.isc.rest.model.queryResult).FromClassQuery("_$$$QUOTE(tResultClass)_","_$$$QUOTE(class)_","_$$$QUOTE(method)_",args...)"
 	} Else {
 		If (returnType = "") {
 			Set methodCall = "Do "
 		} Else {
-			Set methodCall = "Set result = "
+			Set methodCall = "Set "_resultVariableName_" = "
 		}
 		Set methodCall = methodCall_$Case(..target,
 			"instance":"pInstance.",
 			"class":"##class("_class_").")_method_"(args...)"
 	}
 	$$$GENERATE(methodCall)
 	If (returnType = "%Library.Status") {
-		$$$GENERATE("$$$ThrowOnError(result)")
+		$$$GENERATE("$$$ThrowOnError("_resultVariableName_")")
 	} ElseIf (returnType '= "") {
-		If $ClassMethod(returnType,"%Extends","%Library.DynamicAbstractObject") {
-			Set pResultContentType = "application/json"
-			Set exportCommand = "Write result.%ToJSON()"
-		} ElseIf $ClassMethod(returnType,"%Extends","%pkg.isc.rest.model.resource") {
-			Set pResultContentType = $Parameter(returnType,"MEDIATYPE")
-			Set exportCommand = "Do result.JSONExport()"
-		} ElseIf $ClassMethod(returnType,"%Extends","%pkg.isc.rest.model.queryResult") {
-			Set pResultContentType = $Parameter(tResultClass,"MEDIATYPE")
-			Set exportCommand = "$$$ThrowOnError(result.JSONExport())"
-		}ElseIf $ClassMethod(returnType,"%Extends","%Stream.Object") {
-			Set pResultContentType = "application/octet-stream"
-			Set exportCommand = "Do result.OutputToDevice()"
-		} ElseIf $Parameter(returnType,"%JSONENABLED") {
-			Set pResultContentType = "application/json"
-			Set exportCommand = "Do result.%JSONExport()"
-		} Else {
-			$$$GENERATE("// Unknown handling for return type: "_returnType)
-			$$$GENERATE("Set %response.Status = "_$$$QUOTE(##class(%CSP.REST).#HTTP204NOCONTENT))
-			Quit
+		Set exportCommandList = ..ComputeExportCommandList(
+			resultVariableName, returnType, tResultClass, .pResultContentType
+		)
+		If (pResultContentType '= "") {
+			$$$GENERATE("Set %response.ContentType = "_$$$QUOTE(pResultContentType))
+		}
+		$$$GENERATE("If $Data("_resultVariableName_") {")
+		Set ptr = 0
+		While $ListNext(exportCommandList,ptr,exportCommand) {
+			$$$GENERATE("	"_exportCommand)
 		}
-		$$$GENERATE("Set %response.ContentType = "_$$$QUOTE(pResultContentType))
-		$$$GENERATE("If $IsObject(result) {")
-		$$$GENERATE("	"_exportCommand)
 		$$$GENERATE("} Else {")
 		$$$GENERATE("	Set %response.Status = "_$$$QUOTE(##class(%CSP.REST).#HTTP204NOCONTENT))
 		$$$GENERATE("}")
 	}
 }
 
-Method GetFormalSpecMap(ByRef pModelClass As %String, Output pClass, Output pMethod, Output pArgArray, Output pNameMap, Output pReturnType)
+Method ComputeExportCommandList(pResultVariableName As %String, pReturnType As %Dictionary.Classname, pResultClass As %Dictionary.Classname, Output pResultContentType As %String) As %Library.List [ Internal ]
+{
+	Set pResultContentType = ""
+	Set exportCommandList = ""
+	Set classType = $$$comClassKeyGet(pReturnType,$$$cCLASSclasstype)
+	Set unknownReturnTypeError = "Unknown handling for return type: " _ $$$QUOTE(pReturnType) _ " for action " _ $$$QUOTE(..name)
+	If (classType = $$$cCLASSCLASSTYPEDATATYPE) {
+		// Data types
+		If (pReturnType = "%Library.List") {
+			// Specific handling for $ListBuild as a dynamic array
+			Set pResultContentType = ##class(%CSP.REST).#CONTENTTYPEJSON
+			Set exportCommandList = $ListBuild(
+				"Set jsonResult = []",
+				"Set ptr = 0",
+				"While $ListNext("_pResultVariableName_",ptr,value) {",
+				"  Do jsonResult.%Push(value)",
+				"}",
+				"Do "_pResultVariableName_".%ToJSON()"
+			)
+		} Else {
+			// All other data types are simple text to write out
+			Set pResultContentType = ##class(%CSP.REST).#CONTENTTYPETEXT
+			Set exportCommandList = $ListBuild("Write "_pResultVariableName)
+		}
+	} Else {
+		If $System.CLS.IsMthd(pReturnType, "%Extends") {
+			If $ClassMethod(pReturnType, "%Extends", "%Collection.AbstractList") {
+				// Lists
+				$$$ThrowStatus($$$ERROR($$$GeneralError,unknownReturnTypeError))
+			} ElseIf $ClassMethod(pReturnType, "%Extends", "%Collection.AbstractArray") {
+				// Arrays
+				$$$ThrowStatus($$$ERROR($$$GeneralError,unknownReturnTypeError))
+			} Else {
+				// Singular Objects i.e. not collections
+				If $ClassMethod(pReturnType,"%Extends","%Library.DynamicAbstractObject") {
+					Set pResultContentType = "application/json"
+					Set exportCommandList = $ListBuild("Do "_pResultVariableName_".%ToJSON()")
+				} ElseIf $ClassMethod(pReturnType,"%Extends","%pkg.isc.rest.model.resource") {
+					Set pResultContentType = $Parameter(pReturnType,"MEDIATYPE")
+					Set exportCommandList = $ListBuild("Do "_pResultVariableName_".JSONExport()")
+				} ElseIf $ClassMethod(pReturnType,"%Extends","%pkg.isc.rest.model.queryResult") {
+					Set pResultContentType = $Parameter(pResultClass,"MEDIATYPE")
+					Set exportCommandList = $ListBuild("$$$ThrowOnError("_pResultVariableName_".JSONExport())")
+				} ElseIf $ClassMethod(pReturnType,"%Extends","%CSP.Stream") {
+					// MUST be before %Stream.Object as this is a special case of that
+					Set pResultContentType = ""
+					Set exportCommandList = $ListBuild(
+						"Set %response.ContentType = "_pResultVariableName_".ContentType",
+						"Do "_pResultVariableName_".OutputToDevice()"
+					)
+				} ElseIf $ClassMethod(pReturnType,"%Extends","%Stream.Object") {
+					Set pResultContentType = "application/octet-stream"
+					Set exportCommandList = $ListBuild("Do "_pResultVariableName_".OutputToDevice()")
+				} ElseIf $Parameter(pReturnType,"%JSONENABLED") {
+					Set pResultContentType = "application/json"
+					Set exportCommandList = $ListBuild("Do "_pResultVariableName_".%JSONExport()")
+				} Else {
+					$$$ThrowStatus($$$ERROR($$$GeneralError,unknownReturnTypeError))
+				}
+			}
+		} Else {
+			If $Parameter(pReturnType,"%JSONENABLED") {
+				Set pResultContentType = "application/json"
+				Set exportCommandList = $ListBuild("Do "_pResultVariableName_".%JSONExport()")
+			} Else {
+				$$$ThrowStatus($$$ERROR($$$GeneralError,unknownReturnTypeError))
+			}
+		}
+	}
+	Return exportCommandList
+}
+
+Method GetFormalSpecMap(ByRef pModelClass As %String, Output pClass As %Dictionary.Classname, Output pMethod As %Dictionary.Identifier, Output pArgArray, Output pNameMap, Output pReturnType As %Dictionary.Classname) [ Internal ]
 {
 	Kill pClass,pMethod,pArgArray,pNameMap,returnType
 
@@ -399,6 +462,10 @@ Method GetFormalSpecMap(ByRef pModelClass As %String, Output pClass, Output pMet
 			Set pClass = pModelClass
 			Set pMethod = ..query
 		}
+		If '$$$comMemberDefined(pClass,$$$cCLASSquery,pMethod) {
+			Set msg = $$$FormatText("Query '%1' is missing in class '%2' but expected as part of action '%3'",pMethod,pClass,..name)
+			$$$ThrowStatus($$$ERROR($$$GeneralError,msg))
+		}
 
 		Set formalspec = $$$comMemberKeyGet(pClass,$$$cCLASSquery,pMethod,$$$cQUERYformalspecparsed)
 		Set pointer = 0
@@ -419,6 +486,11 @@ Method GetFormalSpecMap(ByRef pModelClass As %String, Output pClass, Output pMet
 			Set pClass = pModelClass
 			Set pMethod = ..call
 		}
+
+		If '$$$comMemberDefined(pClass,$$$cCLASSmethod,pMethod) {
+			Set msg = $$$FormatText("Method '%1' is missing in class '%2' but expected as part of action '%3'",pMethod,pClass,..name)
+			$$$ThrowStatus($$$ERROR($$$GeneralError,msg))
+		}
 
 		Set formalspec = $$$comMemberKeyGet(pClass,$$$cCLASSmethod,pMethod,$$$cMETHformalspecparsed)
 		Set pointer = 0