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

Fix missing marshaller #103

Merged
merged 5 commits into from
Mar 9, 2018
Merged

Conversation

vladbarosan
Copy link

Fixes Azure/azure-sdk-for-go#1161

The same logic applied to the unmarshaller applies here also. If there are embedded types
they might have custom marhsallers which will get called in the embedding type ( which will be incorrect ). So we need to generate on all chain a custom marshaller so the embedded type's one is not used.

@ghost ghost assigned vladbarosan Mar 6, 2018
@ghost ghost added the review label Mar 6, 2018
@@ -141,7 +141,7 @@ else
</text>
}

@if (Model.BaseIsPolymorphic || Model.IsPolymorphic || Model.AllProperties.Any(p => p.ModelType is DictionaryTypeGo))
@if (Model.BaseIsPolymorphic || Model.IsPolymorphic || Model.HasFlattenedFields || Model.AllProperties.Any(p => p.ModelType is DictionaryTypeGo))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcardosos do you think is any reason to keep this here ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you mean in models.go?


In reply to: 172636578 [](ancestors = 172636578)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, somehow posted this. Yes we need this.

I was looking at the old CS where we had tne trick with the alias but we are using this now to populate the discriminator value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question. If model has flattened fields, does it need an unmarshaller? Unmarshaller should only be needed with discriminator stuff

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes , is the same issue. if the embedded type has a custom unmarshaller and the embedding type doesnt, the one from the embedded type will be used on the entire embedding type which will result in deserialization errors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:( but we dont want an unmarshaller just becase it has flattened types, right? only if it is flattened and the flattened stuff has some polymorphism
I would not like to create unmarshallers for everything that has a flattened type

Copy link
Author

@vladbarosan vladbarosan Mar 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct if we can check that all chain of embedded types does not have any polymorphism than we can have an if on that. Opened issue to track this Azure/azure-sdk-for-go#1204

@vladbarosan
Copy link
Author

Will merge after validating the original issue was fixed

@olydis
Copy link
Contributor

olydis commented Mar 7, 2018

🤖 AutoRest automatic publish job 🤖

failed

npm WARN deprecated coffee-script@1.12.7: CoffeeScript on NPM has moved to "coffeescript" (no hyphen)
npm WARN deprecated jade@1.11.0: Jade has been renamed to pug, please install the latest version of pug instead of jade
npm WARN deprecated graceful-fs@3.0.11: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js
npm WARN deprecated transformers@2.1.0: Deprecated, use jstransformer
npm WARN deprecated minimatch@2.0.10: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated minimatch@0.2.14: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated graceful-fs@1.2.3: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js

> autorest@2.0.4245 preinstall C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\node_modules\autorest
> node ./preinstall-check


> uws@9.14.0 install C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\node_modules\uws
> node-gyp rebuild > build_log.txt 2>&1 || exit 0


> dotnet-2.0.0@1.4.4 postinstall C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\node_modules\dotnet-2.0.0
> node -e "/*PostInstall: Installs platform-specific .NET framework */try{require('./dist/app.js')}catch(e){}"


> dotnet-sdk-2.0.0@1.4.4 postinstall C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\node_modules\dotnet-sdk-2.0.0
> node -e "/*PostInstall: Installs platform-specific .NET framework */try{require('./dist/app.js')}catch(e){}"


> golang-dep@0.1.4 postinstall C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\node_modules\golang-dep
> node acquire.js

'chmod' is not recognized as an internal or external command,
operable program or batch file.

> @microsoft.azure/autorest.go@2.1.0 prepare C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go
> gulp build

[23:12:52] Using gulpfile C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\gulpfile.js
[23:12:52] Starting 'init'...
[23:12:52] Finished 'init' after 281 μs
[23:12:52] Starting 'clear-cache-on-force'...
[23:12:52] Finished 'clear-cache-on-force' after 30 μs
[23:12:52] Starting 'restore'...
[23:12:52] Starting 'version-number'...
           C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: dotnet restore C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\src\autorest.go.csproj /nologo
[23:12:53] Finished 'version-number' after 732 ms
[23:12:56] Finished 'restore' after 3.8 s
[23:12:56] Starting 'build/dotnet'...
           C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: dotnet build -c Debug C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/autorest.go.sln /nologo /clp:NoSummary /p:VersionPrefix=2.1.92
           C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: dotnet publish -c Debug C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/src/ --output C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/src//bin/netcoreapp2.0 /nologo /clp:NoSummary /p:VersionPrefix=2.1.92
[23:13:13] Finished 'build/dotnet' after 16 s
[23:13:13] Starting 'build'...
Building project in C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go
[23:13:13] Finished 'build' after 216 μs
npm notice created a lockfile as package-lock.json. You should commit this file.
added 548 packages in 66.703s

> @microsoft.azure/autorest.go@2.1.0 publish-preview C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go
> gulp publish-preview

[23:13:18] Using gulpfile C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go\gulpfile.js
[23:13:18] Starting 'init'...
[23:13:18] Finished 'init' after 256 μs
[23:13:18] Starting 'version-number'...
[23:13:18] Starting 'get-tag'...
[23:13:18] Starting 'clear-cache-on-force'...
[23:13:18] Finished 'clear-cache-on-force' after 15 μs
[23:13:18] Starting 'restore'...
[23:13:18] Finished 'restore' after 546 ms
[23:13:18] Finished 'version-number' after 639 ms
[23:13:18] Starting 'build/dotnet'...
           C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: dotnet build -c Debug C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/autorest.go.sln /nologo /clp:NoSummary /p:VersionPrefix=2.1.92
[23:13:18] Finished 'get-tag' after 655 ms
           C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: dotnet publish -c Debug C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/src/ --output C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/src//bin/netcoreapp2.0 /nologo /clp:NoSummary /p:VersionPrefix=2.1.92
[23:13:30] Finished 'build/dotnet' after 12 s
[23:13:30] Starting 'build'...
Building project in C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go
[23:13:30] Finished 'build' after 172 μs
[23:13:30] Starting 'publish-preview'...
           C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/node_modules/.bin/yarn publish --tag preview --new-version 2.1.92 --access public --no-git-tag-version


Published:  @microsoft.azure/autorest.go@2.1.92 (tagged as @preview)


Exec Failed C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go :: git checkout C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/.gitignore
(stderr)
  fatal: C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/.gitignore: 'C:\Users\AUTORE~1\AppData\Local\Temp\2\PUBLISHedxhk\103_20180309T231201\autorest.go/.gitignore' is outside repository



Task Failed:  Execute Task failed, fast exit

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @microsoft.azure/autorest.go@2.1.0 publish-preview: `gulp publish-preview`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @microsoft.azure/autorest.go@2.1.0 publish-preview script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\autorest-ci\AppData\Roaming\npm-cache\_logs\2018-03-09T23_13_54_771Z-debug.log

@olydis
Copy link
Contributor

olydis commented Mar 9, 2018

🤖 AutoRest automatic feature coverage report 🤖

feature set version 2.4.2

❌️ General: 91%

45 out of 535 features are not covered by tests

HttpClientFailure429Head, HttpRedirect300Get, HttpRedirect301Put, HttpRedirect302Patch, HttpRedirect307Delete, HttpRedirect307Patch, HttpRedirect307Post, HttpRedirect307Put, UrlPathsArrayCSVInPath, UrlPathsIntUnixTime, UrlPathsStringBase64Url, UrlQueriesArrayCsvEmpty, UrlQueriesArrayMultiEmpty, UrlQueriesArrayMultiNull, UrlQueriesArrayMultiValid, additionalPropertiesInProperties, additionalPropertiesInPropertiesWithAPTypeString, additionalPropertiesTrue, additionalPropertiesTypeObject, additionalPropertiesTypeString, allowedValueEnum, animalNotFoundError, expectedEnum, expectedNoErrors, expectedPetHungryError, expectedPetSadError, getArrayBase64Url, getDictionaryBase64Url, getEnumReferenced, getEnumReferencedConstant, getStringBase64Encoded, getStringBase64UrlEncoded, getStringNullBase64UrlEncoding, intError, linkNotFoundError, putComplexArrayValid, putComplexPolymorphismComplicated, putComplexPolymorphismNoDiscriminator, putComplexReadOnlyPropertyValid, putEnumReferenced, putEnumReferencedConstant, putStringBase64UrlEncoded, roundTripEnum, stringError, unexpectedEnum

❌️ Azure: 11%

105 out of 118 features are not covered by tests

AzureApiVersionMethodGlobalNotProvidedValid, AzureApiVersionMethodGlobalValid, AzureApiVersionMethodLocalNull, AzureApiVersionMethodLocalValid, AzureApiVersionPathGlobalValid, AzureApiVersionPathLocalValid, AzureApiVersionSwaggerGlobalValid, AzureApiVersionSwaggerLocalValid, AzureMethodPathUrlEncoding, AzureMethodQueryUrlEncoding, AzureMethodQueryUrlEncodingNull, AzureODataFilter, AzurePathPathUrlEncoding, AzurePathQueryUrlEncoding, AzureRequestClientIdInError, AzureSubscriptionMethodGlobalNotProvidedValid, AzureSubscriptionMethodGlobalValid, AzureSubscriptionMethodLocalValid, AzureSubscriptionPathGlobalValid, AzureSubscriptionPathLocalValid, AzureSubscriptionSwaggerGlobalValid, AzureSubscriptionSwaggerLocalValid, AzureSwaggerPathUrlEncoding, AzureSwaggerQueryUrlEncoding, AzureXmsCustomNamedRequestId, AzureXmsCustomNamedRequestIdParameterGroup, AzureXmsRequestClientIdNull, AzureXmsRequestClientOverwrite, AzureXmsRequestClientOverwriteViaParameter, CustomHeaderPostAsyncSucceded, CustomHeaderPostSucceeded, CustomHeaderPutAsyncSucceded, CustomHeaderPutSucceeded, LRODelete200, LRODeleteAsyncNoHeaderInRetry, LRODeleteAsyncNoRetrySucceeded, LRODeleteAsyncRetryCanceled, LRODeleteAsyncRetryFailed, LRODeleteAsyncRetrySucceeded, LRODeleteInlineComplete, LRODeleteNoHeaderInRetry, LRODeleteProvisioningCanceled, LRODeleteProvisioningFailed, LRODeleteProvisioningSucceededWithBody, LROErrorDelete202RetryInvalidHeader, LROErrorDeleteAsyncInvalidHeader, LROErrorDeleteAsyncInvalidJsonPolling, LROErrorDeleteAsyncNoPollingStatus, LROErrorDeleteNoLocation, LROErrorPost202RetryInvalidHeader, LROErrorPostAsyncInvalidHeader, LROErrorPostAsyncInvalidJsonPolling, LROErrorPostAsyncNoPollingPayload, LROErrorPostNoLocation, LROErrorPut200InvalidJson, LROErrorPut201NoProvisioningStatePayload, LROErrorPutAsyncInvalidHeader, LROErrorPutAsyncInvalidJsonPolling, LROErrorPutAsyncNoPollingStatus, LROErrorPutAsyncNoPollingStatusPayload, LRONonRetryDelete400, LRONonRetryDeleteAsyncRetry400, LRONonRetryPost202Retry400, LRONonRetryPost400, LRONonRetryPostAsyncRetry400, LRONonRetryPut201Creating400, LRONonRetryPut201Creating400InvalidJson, LRONonRetryPut400, LRONonRetryPutAsyncRetry400, LROPost200, LROPostAsyncNoRetrySucceeded, LROPostAsyncRetryCanceled, LROPostAsyncRetryFailed, LROPostAsyncRetrySucceeded, LROPostSuccededNoBody, LROPostSuccededWithBody, LROPut200InlineCompleteNoState, LROPut202Retry200, LROPutAsyncNoHeaderInRetry, LROPutAsyncNoRetryCanceled, LROPutAsyncNoRetrySucceeded, LROPutAsyncRetryFailed, LROPutAsyncRetrySucceeded, LROPutCanceled, LROPutFailed, LROPutInlineComplete, LROPutNoHeaderInRetry, LROPutNonResourceAsyncInRetry, LROPutNonResourceInRetry, LROPutSubResourceAsyncInRetry, LROPutSubResourceInRetry, LROPutSucceededNoBody, LROPutSucceededWithBody, LRORetryErrorDelete202Accepted200Succeeded, LRORetryErrorDeleteAsyncRetrySucceeded, LRORetryErrorPost202Retry200Succeeded, LRORetryErrorPostAsyncRetrySucceeded, LRORetryErrorPutAsyncSucceeded, LRORetryErrorPutAsyncSucceededPolling, LRORetryPutSucceededWithBody, SubscriptionIdAndApiVersion, postParameterGroupingMultipleParameterGroups, postParameterGroupingOptionalParameters, postParameterGroupingRequiredParameters, postParameterGroupingSharedParameterGroupObject

@vladbarosan vladbarosan merged commit 99b86f4 into Azure:master Mar 9, 2018
@ghost ghost removed the review label Mar 9, 2018
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.

4 participants