Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generator-go-sdk: compatiable with literal string response #1756

Closed
wants to merge 2 commits into from

Conversation

wuxu92
Copy link
Collaborator

@wuxu92 wuxu92 commented Oct 26, 2022

If the Response Body is a literal string, not a structure JSON, the currently generated code will also unmarshal by JSON which causes an error. This PR tries to unmarshalBytes for this kind of response. So we don't need to wait service team to fix the swagger issue Azure/azure-sdk-for-go#17591 and we can solve all failed Automation AccTest cases.

Makes these PRs possible or improved

TODO

remove test code in preparerForList for content type (autorest.AsContentType("text/powershell"), once #1908 merged.

Diff of generated code

I have generated all RPs locally, and here is the diff between the code by the new template and the main branch of hashicorp/go-azure-sdk

diff --git a/resource-manager/automation/2019-06-01/dscconfiguration/method_getcontent_autorest.go b/resource-manager/automation/2019-06-01/dscconfiguration/method_getcontent_autorest.go
index 1af9f4fa..c136f91e 100644
--- a/resource-manager/automation/2019-06-01/dscconfiguration/method_getcontent_autorest.go
+++ b/resource-manager/automation/2019-06-01/dscconfiguration/method_getcontent_autorest.go
@@ -58,11 +58,14 @@ func (c DscConfigurationClient) preparerForGetContent(ctx context.Context, id Co
 // responderForGetContent handles the response to the GetContent request. The method always
 // closes the http.Response Body.
 func (c DscConfigurationClient) responderForGetContent(resp *http.Response) (result GetContentOperationResponse, err error) {
+	var content []byte
 	err = autorest.Respond(
 		resp,
 		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByUnmarshallingJSON(&result.Model),
+		autorest.ByUnmarshallingBytes(&content),
 		autorest.ByClosing())
+	str := string(content)
+	result.Model = &str
 	result.HttpResponse = resp
 
 	return
diff --git a/resource-manager/automation/2019-06-01/job/method_getoutput_autorest.go b/resource-manager/automation/2019-06-01/job/method_getoutput_autorest.go
index e4c1838b..48c601a8 100644
--- a/resource-manager/automation/2019-06-01/job/method_getoutput_autorest.go
+++ b/resource-manager/automation/2019-06-01/job/method_getoutput_autorest.go
@@ -87,11 +87,14 @@ func (c JobClient) preparerForGetOutput(ctx context.Context, id JobId, options G
 // responderForGetOutput handles the response to the GetOutput request. The method always
 // closes the http.Response Body.
 func (c JobClient) responderForGetOutput(resp *http.Response) (result GetOutputOperationResponse, err error) {
+	var content []byte
 	err = autorest.Respond(
 		resp,
 		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByUnmarshallingJSON(&result.Model),
+		autorest.ByUnmarshallingBytes(&content),
 		autorest.ByClosing())
+	str := string(content)
+	result.Model = &str
 	result.HttpResponse = resp
 
 	return
diff --git a/resource-manager/automation/2019-06-01/job/method_getrunbookcontent_autorest.go b/resource-manager/automation/2019-06-01/job/method_getrunbookcontent_autorest.go
index d906300d..5960a471 100644
--- a/resource-manager/automation/2019-06-01/job/method_getrunbookcontent_autorest.go
+++ b/resource-manager/automation/2019-06-01/job/method_getrunbookcontent_autorest.go
@@ -87,11 +87,14 @@ func (c JobClient) preparerForGetRunbookContent(ctx context.Context, id JobId, o
 // responderForGetRunbookContent handles the response to the GetRunbookContent request. The method always
 // closes the http.Response Body.
 func (c JobClient) responderForGetRunbookContent(resp *http.Response) (result GetRunbookContentOperationResponse, err error) {
+	var content []byte
 	err = autorest.Respond(
 		resp,
 		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByUnmarshallingJSON(&result.Model),
+		autorest.ByUnmarshallingBytes(&content),
 		autorest.ByClosing())
+	str := string(content)
+	result.Model = &str
 	result.HttpResponse = resp
 
 	return
diff --git a/resource-manager/automation/2019-06-01/runbook/method_getcontent_autorest.go b/resource-manager/automation/2019-06-01/runbook/method_getcontent_autorest.go
index 22370327..2bb7cbca 100644
--- a/resource-manager/automation/2019-06-01/runbook/method_getcontent_autorest.go
+++ b/resource-manager/automation/2019-06-01/runbook/method_getcontent_autorest.go
@@ -58,11 +58,14 @@ func (c RunbookClient) preparerForGetContent(ctx context.Context, id RunbookId)
 // responderForGetContent handles the response to the GetContent request. The method always
 // closes the http.Response Body.
 func (c RunbookClient) responderForGetContent(resp *http.Response) (result GetContentOperationResponse, err error) {
+	var content []byte
 	err = autorest.Respond(
 		resp,
 		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByUnmarshallingJSON(&result.Model),
+		autorest.ByUnmarshallingBytes(&content),
 		autorest.ByClosing())
+	str := string(content)
+	result.Model = &str
 	result.HttpResponse = resp
 
 	return
diff --git a/resource-manager/automation/2019-06-01/runbookdraft/method_getcontent_autorest.go b/resource-manager/automation/2019-06-01/runbookdraft/method_getcontent_autorest.go
index 15814ca0..c75b0910 100644
--- a/resource-manager/automation/2019-06-01/runbookdraft/method_getcontent_autorest.go
+++ b/resource-manager/automation/2019-06-01/runbookdraft/method_getcontent_autorest.go
@@ -58,11 +58,14 @@ func (c RunbookDraftClient) preparerForGetContent(ctx context.Context, id Runboo
 // responderForGetContent handles the response to the GetContent request. The method always
 // closes the http.Response Body.
 func (c RunbookDraftClient) responderForGetContent(resp *http.Response) (result GetContentOperationResponse, err error) {
+	var content []byte
 	err = autorest.Respond(
 		resp,
 		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByUnmarshallingJSON(&result.Model),
+		autorest.ByUnmarshallingBytes(&content),
 		autorest.ByClosing())
+	str := string(content)
+	result.Model = &str
 	result.HttpResponse = resp
 
 	return
diff --git a/resource-manager/postgresql/2021-06-01/getprivatednszonesuffix/method_execute_autorest.go b/resource-manager/postgresql/2021-06-01/getprivatednszonesuffix/method_execute_autorest.go
index 57464513..2100294a 100644
--- a/resource-manager/postgresql/2021-06-01/getprivatednszonesuffix/method_execute_autorest.go
+++ b/resource-manager/postgresql/2021-06-01/getprivatednszonesuffix/method_execute_autorest.go
@@ -57,11 +57,14 @@ func (c GetPrivateDnsZoneSuffixClient) preparerForExecute(ctx context.Context) (
 // responderForExecute handles the response to the Execute request. The method always
 // closes the http.Response Body.
 func (c GetPrivateDnsZoneSuffixClient) responderForExecute(resp *http.Response) (result ExecuteOperationResponse, err error) {
+	var content []byte
 	err = autorest.Respond(
 		resp,
 		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByUnmarshallingJSON(&result.Model),
+		autorest.ByUnmarshallingBytes(&content),
 		autorest.ByClosing())
+	str := string(content)
+	result.Model = &str
 	result.HttpResponse = resp
 
 	return

May affect RPs:

image

@hashicorp-cla
Copy link

hashicorp-cla commented Oct 26, 2022

CLA assistant check
All committers have signed the CLA.

@wuxu92
Copy link
Collaborator Author

wuxu92 commented Nov 10, 2022

Hi @tombuildsstuff how do you think of this Fix? it will fix some problems of Automation RP. is there a thing I should do to make it merge?

@tombuildsstuff
Copy link
Contributor

@wuxu92 apologies for the delay, we'll need to update this to check the Content-Type that's being threaded through in #1760 (conditionally unmarshaling as JSON/or assigning the string value) - so once that's merged we can look into using that, which'll fix this - I'm hoping to merge that one shortly fwiw.

@wuxu92
Copy link
Collaborator Author

wuxu92 commented Nov 21, 2022

@tombuildsstuff thanks for your update! I tested with the Content-type logic locally, the produces property in swagger is text/powershell, seems there is no easy way to check all types?

will you create a new PR to fix this using Content-type logic?

@tombuildsstuff
Copy link
Contributor

Superseded by #2332 - which'll ultimately be fixed by hashicorp/go-azure-sdk#298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants