-
Notifications
You must be signed in to change notification settings - Fork 166
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
Remove extra method generated by "x-ms-pageable" custom nextlink operation #1056
Comments
chamons
added
the
bug
This issue requires a change to an existing behavior in the product in order to be resolved.
label
May 27, 2021
AlexanderSher
added a commit
to AlexanderSher/autorest.csharp
that referenced
this issue
May 29, 2021
…om nextlink operation
6 tasks
chamons
added a commit
that referenced
this issue
Jun 9, 2021
…e methods (#1303) # Description This is a second attempt at #1281, with a bit of a deeper dig into how we ran into the situtation. Pavel pointed out during a discussion that we really shouldn't be seeing any of these "extra" methods at the level #1281 was filtering them out. After applying industrial strength bisect I was able to come up with this test case: ```yaml !CodeModel operationGroups: - !OperationGroup operations: - !Operation language: !Languages default: paging: nextLinkName: nextLink - !Operation language: !Languages default: paging: nextLinkName: nextLink nextLinkOperation: !Operation &ref_17 language: !Languages default: paging: nextLinkName: nextLink nextLinkOperation: *ref_17 - *ref_17 ``` In it this code: ```csharp foreach (var op in v.OperationGroups.First().Operations) { var paging = op.Language.Default.Paging; bool hasNextOperation = paging?.NextLinkOperation != null; ``` would return true for the second and third operation in my simplified model but the third (*ref_17) would differ in the "full" autorest.csharp model. The culprit was surprisingly: ``` - internal partial class Language : IDictionary<string, object> + internal partial class Language ``` For some, honestly unknown, reason YamlDotNet would not handle this case correctly as long as we implemented the dictionary interface. I decided against deep diving into that 3rd party dependency to understand this further. This already cost too many days. I did try bumping to latest, which failed for unrelated reasons but did not change this behavior. Removing the IDictionary interface required adding a few fields (header, summary) that were being indirectly handled, and may require more when I test this with azure-sdk-for-net this afternoon. Test case included for #1056 in particular. The removals in the paging test suites are expected.
Fixed by #1303 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For example we have this customize next link operation
NextFragment
defined in the swagger. It generates NextFragment() operation which is correct but it also generates NextFragmentNextPage() operation which I think is not needed. We can consider removing it if there is no usecase of this extra operation.The text was updated successfully, but these errors were encountered: