Merge pull request #19 from intersystems/internal-updates

Release 1.2.1

CHANGELOG.md

@@ -5,6 +5,15 @@ 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).
 
+## [1.2.1] - 2022-10-05
+
+### Fixed
+- APPS-15075: Generating OpenAPI doc produces deterministic results
+- APPS-13794: OpenAPI generated properly for query/path action parameters
+- APPS-13849: OpenAPI doc includes query filter
+- APPS-13664: %Dictionary.Classname is treated as string instead of object
+- Unsupported actions are always omitted from OpenAPI generation
+
 ## [1.2.0] - 2022-08-08
 
 ### Added 

cls/_pkg/isc/rest/openAPI.cls

@@ -683,7 +683,7 @@ Method GetWebApplicationInfo() [ Internal, Private ]
 Method CollectResourceInfo() [ Internal, Private ]
 {
 	// Get resource info from the %pkg.isc.rest.resourceMap table
-	Set query = "select MediaType, ResourceClass, ResourceName from %pkg_isc_rest.resourceMap where DispatchClass = ?"
+	Set query = "select MediaType, ResourceClass, ResourceName from %pkg_isc_rest.resourceMap where DispatchClass = ? order by ResourceClass"
 	Set result = ##class(%SQL.Statement).%ExecDirect(,query,..DispatchClass)
 	If result.%SQLCODE < 0 {
 		Throw ##class(%Exception.SQL).CreateFromSQLCODE(result.%SQLCODE, result.%Message)
@@ -736,7 +736,7 @@ Method CollectResourceInfo() [ Internal, Private ]
 Method CollectActionInfo() [ Internal ]
 {
 	// Get action info from the %pkg.isc.rest.actionMap table
-	Set query = "select AcceptsOrNUL, ActionName, ActionTarget, HTTPVerb, ImplementationClass, MediaTypeOrNUL, ResourceClass, ResourceName from %pkg_isc_rest.actionMap where DispatchClass = ?"
+	Set query = "select AcceptsOrNUL, ActionName, ActionTarget, HTTPVerb, ImplementationClass, MediaTypeOrNUL, ResourceClass, ResourceName from %pkg_isc_rest.actionMap where DispatchClass = ? order by ResourceClass, ActionName, HTTPVerb"
 	Set result = ##class(%SQL.Statement).%ExecDirect(,query,..DispatchClass)
 	If result.%SQLCODE < 0 {
 		Throw ##class(%Exception.SQL).CreateFromSQLCODE(result.%SQLCODE, result.%Message)
@@ -1023,7 +1023,7 @@ Method WriteResourceEndpoints() [ Internal, Private ]
 				// Parameter generation
 				try {
 					// The "nice" way to do this: these are the properties that are reported as being OK to query as reported by DBMappedResource
-					Set parametersIterator = $ClassMethod(resourceSchema.ClassName, "getProxyColumnList").%GetIterator()
+					Set parametersIterator = $ClassMethod(resourceSchema.ClassName, "GetProxyColumnList").%GetIterator()
 				} catch (ex) {
 					// TODO: Come up with some way to handle classes that don't use DBMappedResource methods, instead of (incorrectly) listing them as having no parameters.
 					// TODO: The following methods theoretically perform known operations:
@@ -1503,7 +1503,7 @@ Method WriteActionEndpoints() [ Internal, Private ]
 	For i=1:1:..ActionInfo.Count() {
 		#dim actionSchema As %pkg.isc.rest.openAPI.actionInfo
 		Set actionSchema = ..ActionInfo.GetAt(i)
-		If ('actionSchema.Supported) && ((actionSchema.Forbidden && actionSchema.ForbidUnderAllCircumstances) ||
+		If ('actionSchema.Supported) || ((actionSchema.Forbidden && actionSchema.ForbidUnderAllCircumstances) ||
 		   ('..IncludeForbiddenEndpoints && '..ForceAuthorizeAllEndpoints && actionSchema.Forbidden)) {
 			Continue
 		}
@@ -1576,35 +1576,36 @@ Method WriteActionEndpoints() [ Internal, Private ]
 				// Query/Path Parameters
 				#Dim parameter As %pkg.isc.rest.openAPI.model.parameter
 				Set parameter = ""
+				Set found = 0
 				For j=1:1:methodObj.Parameters.Count() {
 					Set existingParameter = methodObj.Parameters.GetAt(j)
 					If existingParameter.Name = argument.Name && (existingParameter.In = argument.Source) {
 						Set parameter = methodObj.Parameters.GetAt(j)
+						Set found = 1
 						Quit
 					}
 				}
 				Set schema = argument.Schema
-				Set mediatype = ##class(%pkg.isc.rest.openAPI.model.mediaType).%New()
-				Do mediatype.SourceClasses.Insert(argument.SourceClass)
-				Set mediatype.Schema = schema
 				If parameter = "" {
 					Set parameter = ##class(%pkg.isc.rest.openAPI.model.parameter).%New()
-					Do parameter.Content.SetAt(mediatype, argument.MediaType)
+					Set parameter.Schema = schema
 				} Else {
-					Set existingSchema = parameter.Content.GetNext("")
-					If $IsObject(existingSchema) {
-						Set baseSchema = ##class(%pkg.isc.rest.openAPI.model.schema).%New()
-						Set baseSchema.AutoGenerated = 1
-						For j=1:1:existingSchema.SourceClasses.Count() {
-							Do baseSchema.SourceClasses.Insert(existingSchema.SourceClasses.GetAt(j))
+					Set baseSchema = parameter.Schema
+
+					// If we have the same action from multiple sources,
+					// don't add duplicate "OneOf" entries with the same reference.
+					// (Could do deduplication downstream too.)
+					Set foundRef = 0
+					If (schema.Ref '= "") {
+						For j=1:1:baseSchema.OneOf.Count() {
+							If baseSchema.OneOf.GetAt(j).Ref = schema.Ref {
+								Set foundRef = 1
+							}
 						}
-						Do parameter.Content.Clear()
-						Set parameter.Schema = baseSchema
-						Do baseSchema.OneOf.Insert(existingSchema)
-					} Else {
-						Set baseSchema = parameter.Schema
 					}
-					Do baseSchema.OneOf.Insert(schema)
+					If 'foundRef {
+						Do baseSchema.OneOf.Insert(schema)
+					}
 					For j=1:1:schema.SourceClasses.Count() {
 						Do baseSchema.SourceClasses.Insert(schema.SourceClasses.GetAt(j))
 					}
@@ -1615,7 +1616,9 @@ Method WriteActionEndpoints() [ Internal, Private ]
 				//Set parameter.Description = "TODO: ???"
 				// Path parameters are always required
 				Set parameter.Required = argument.Required || (argument.Source = "path")
-				Do methodObj.Parameters.Insert(parameter)
+				If 'found {
+					Do methodObj.Parameters.Insert(parameter)
+				}
 			} ElseIf argument.Source = "body-key" {
 				// Body-Key Parameters
 				Set requestBody = methodObj.RequestBody
@@ -1983,4 +1986,3 @@ Method WriteSection(text, ByRef ws, ByRef timer) [ Internal, Private ]
 }
 
 }
-

cls/_pkg/isc/rest/openAPI/util.cls

@@ -260,8 +260,12 @@ ClassMethod GetJSONType(className As %String) As %String
 		} ElseIf $ClassMethod(className, "%Extends", "%Stream.Object") {
 			// TODO: Support binary streams as NOT strings...
 			Set jsonType = "string"
+		} ElseIf (xsdType '= "") {
+			Set jsonType = $Select(xsdType="long":"integer",xsdType="boolean":"boolean",$Match(xsdType,"decimal|double|short|byte"):"number",1:"string")
+		} ElseIf ($$$comClassKeyGet(className,$$$cCLASSclasstype) = $$$cCLASSCLASSTYPEDATATYPE) {
+			Set jsonType = "string"
 		} Else {
-			Set jsonType = $Select(xsdType="":"object",xsdType="long":"integer",xsdType="boolean":"boolean",$Match(xsdType,"decimal|double|short|byte"):"number",1:"string")
+			Set jsonType = "object"
 		}
 	}
 	Set:className="%Library.DynamicArray" jsonType = "array"
@@ -339,4 +343,3 @@ ClassMethod ReadClassMethodOutput(className, methodName) As %String [ Internal ]
 }
 
 }
-

internal/testing/unit_tests/UnitTest/isc/rest/openAPI.cls

@@ -440,7 +440,7 @@ Method TestGenerationWithSharedResourceName()
 	// /unittest-resource/$test-action checks
 	#Dim actionPathItem As %pkg.isc.rest.openAPI.model.pathItem
 	Set actionPathItem = openapi.Specification.Paths.GetAt("/unittest-resource/$test-action")
-	Do $$$AssertEquals(actionPathItem.SourceClasses.Count(), 2, "Action path item has only one source class")
+	Do $$$AssertEquals(actionPathItem.SourceClasses.Count(), 1, "Action path item has only one source class")
 	Do $$$AssertEquals(actionPathItem.SourceClasses.GetAt(1), "zUnitTest.isc.rest.class3", "Action path item has the correct source class")
 	Do $$$AssertEquals(actionPathItem.Get, "", "Action GET operation is not set")
 	Do $$$AssertEquals(actionPathItem.Put, "", "Action PUT operation is not set")
@@ -449,6 +449,15 @@ Method TestGenerationWithSharedResourceName()
 	Do $$$AssertEquals(actionPathItem.Trace, "", "Action TRACE operation is not set")
 	Do $$$AssertEquals(actionPathItem.Head, "", "Action HEAD operation is not set")
 	Do $$$AssertEquals(actionPathItem.Options, "", "Action OPTIONS operation is not set")
+	Do $$$AssertEquals(actionPathItem.Post.Tags.GetAt(1),"unittest-resource")
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.Count(),2)
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.GetAt(1).In,"query")
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.GetAt(1).Name,"op")
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.GetAt(1).Schema.Ref,"#/components/schemas/string_input")
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.GetAt(2).In,"query")
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.GetAt(2).Name,"uc")
+	Do $$$AssertEquals(actionPathItem.Post.Parameters.GetAt(2).Schema.Ref,"#/components/schemas/registered-object_input")
+	Do $$$AssertTrue($IsObject(actionPathItem.Post.RequestBody.Content.GetAt("application/json")))
 
 	// /unittest-resource/{id} checks
 	#Dim resourceInstancePathItem As %pkg.isc.rest.openAPI.model.pathItem
@@ -1223,6 +1232,26 @@ Method TestPathParameterActions()
 	}
 }
 
+Method TestQueryFilters()
+{
+	Set openapi = ..SetupTestingEnvironment()
+	#Dim openapi As %pkg.isc.rest.openAPI
+	Set openapi.DEBUG = 0
+	Do $$$AssertStatusOK(..SetupClass("zUnitTest.isc.rest.class1", ["%Persistent","%pkg.isc.rest.model.adaptor"], {"RESOURCENAME":"unittest-resource"}, "Demo",{"Type":"%String"},{"%JSONFIELDNAME":"demo"}, ,,, , ["CheckPermissionAllowAll:CheckPermission","GetCollectionCustom:GetCollection"]))
+	Do $$$AssertStatusOK(..CompileClass("zUnitTest.isc.rest.class1"))
+	Do $$$AssertTrue(openapi.GetSpecificationI($$$NULLOREF, "/UnitTest/isc/rest/openAPI/api/"), "Specification generation completed without errors")
+
+	Do openapi.Specification.%JSONExportToString(.str)
+	Do $$$LogMessage(str)
+
+	Set onlySearchParam = openapi.Specification.Paths.GetAt("/unittest-resource").Get.Parameters.GetAt(1)
+	Do $$$AssertTrue($IsObject(onlySearchParam))
+	Do $$$AssertEquals(onlySearchParam.In,"query")
+	Do $$$AssertEquals(onlySearchParam.Name,"demo")
+	Do $$$AssertEquals(onlySearchParam.Style,"deepObject")
+	Do $$$AssertEquals(onlySearchParam.Schema.Ref,"#/components/schemas/QueryFilter")
+}
+
 Method TestMultiHandlers()
 {
 	Do $$$AssertStatusOK(..SetupClass("zUnitTest.isc.rest.handler1", ["%pkg.isc.rest.handler"],, ,,, ,,, , ["AuthenticationStrategy","CheckResourcePermitted"]))
@@ -1498,4 +1527,3 @@ XData IntegerIdentityActionMap [ XMLNamespace = "http://www.intersystems.com/_pk
 }
 
 }
-

internal/testing/unit_tests/UnitTest/isc/rest/openAPI/util.cls

@@ -41,6 +41,34 @@ Method TestGetJSON()
 	Do ..TeardownClass("zUnitTest.isc.rest.class1")
 }
 
+Method TestGetJSONType()
+{
+	#Define GetJSONType ##class(%pkg.isc.rest.openAPI.util).GetJSONType
+
+	// Objects
+	Do $$$AssertEquals($$$GetJSONType("%RegisteredObject"),"object")
+	Do $$$AssertEquals($$$GetJSONType("%SystemBase"),"object")
+	Do $$$AssertEquals($$$GetJSONType($classname()),"object")
+
+	// Normal datatypes
+	Do $$$AssertEquals($$$GetJSONType("%String"),"string")
+	Do $$$AssertEquals($$$GetJSONType("%Binary"),"string")
+	Do $$$AssertEquals($$$GetJSONType("%Numeric"),"number")
+	Do $$$AssertEquals($$$GetJSONType("%Integer"),"number")
+	Do $$$AssertEquals($$$GetJSONType("%Float"),"number")
+	Do $$$AssertEquals($$$GetJSONType("%Double"),"double")
+	Do $$$AssertEquals($$$GetJSONType("%Date"),"string")
+	Do $$$AssertEquals($$$GetJSONType("%Boolean"),"boolean")
+
+	// %List is treated as a string; we have a special datatype to project as array
+	Do $$$AssertEquals($$$GetJSONType("%List"),"string")
+	Do $$$AssertEquals($$$GetJSONType("%pkg.isc.json.dataType.list"),"array")
+
+	// Special cases: datatype classes without %JSONTYPE are strings
+	Do $$$AssertEquals($$$GetJSONType("%Dictionary.Classname"),"string")
+	Do $$$AssertEquals($$$GetJSONType("%Dictionary.CacheClassname"),"string")
+}
+
 Method TestFromJSON()
 {
 	#define JSONAdaptor ##class(UnitTest.isc.rest.openAPI.compatibility).GetJSONAdaptorClass()
@@ -76,4 +104,3 @@ ClassMethod SampleMethod()
 }
 
 }
-

module.xml

@@ -2,7 +2,7 @@
 <Document name="isc.rest.ZPM">
     <Module>
       <Name>isc.rest</Name>
-      <Version>1.2.0</Version>
+      <Version>1.2.1</Version>
       <Packaging>module</Packaging>
       <Dependencies>
         <ModuleReference>