Closed kennethlong closed 2 years ago
The parameter IPageable is already passed in the query and we extract with https://github.com/jhipster/jhipster-net/blob/main/src/JHipsterNet.Web/Pagination/Extensions/QueryStringExtensions.cs
This class convert simple param to https://github.com/jhipster/jhipster-net/blob/main/src/JHipsterNet.Core/Pagination/IPageable.cs
I need more invesgation to undestand the bug.
Let me know if you need me to do anything on my side. My workaround has me working for now.
This appears to be a Swagger issue. Did the Swagger ever work for the paginated GET calls? I am looking for a solution and I will share my findings here.
This appears to be a Swagger issue. Did the Swagger ever work for the paginated GET calls?
I'm not really sure but i think yes
can you try this :
in SwaggerStartup.cs
c.OperationFilter<PageableModelFilter>();
and add this class
public class PageableModelFilter : IOperationFilter
{
public void Apply(OpenApiOperation operation, OperationFilterContext context)
{
var description = context.ApiDescription;
if (description.HttpMethod != null && description.HttpMethod.ToLower() != HttpMethod.Get.ToString().ToLower())
{
// We only want to do this for GET requests, if this is not a
// GET request, leave this operation as is, do not modify
return;
}
if (description.ParameterDescriptions.Any(elt => elt.Type == typeof(IPageable)))
{
operation?.RequestBody?.Content?.Clear();
}
if (operation == null)
{
return;
}
operation.Parameters.Add(new OpenApiParameter
{
Name = "page", In = ParameterLocation.Query, Schema = new OpenApiSchema {Type = "number"}
});
operation.Parameters.Add(new OpenApiParameter
{
Name = "size", In = ParameterLocation.Query, Schema = new OpenApiSchema {Type = "number"}
});
}
}
I wrote this quickly but it seems to work well, it can probably be improved
if it's work for you, maybe you can provide an pull request ?
Yes this works, thank you! I was working on a similar approach, but having trouble getting "sort" parameter working in Swagger. I am trying to add sort parameter to PageableModelFilter.
I will keep digging and see if I can figure it out. I will do a pull request when I get this sort part figured out.
You can get sort parameter name here : https://github.com/jhipster/jhipster-net/blob/main/src/JHipsterNet.Core/Pagination/Extensions/PageableBinderConfig.cs
Yes, I found that.
I think the issue is that all the OpenApi stuff tries to map it as "code=value", and we are using a comma separated list of values. I couldn't find any examples that take a set of properties and convert them to a comma separated list of values, so I just went with:
operation.Parameters.Add(new OpenApiParameter { Name = "sort", In = ParameterLocation.Query, Schema = new OpenApiSchema() { Type = "string" } });
(Not sure why it formatted it like that... maybe because I cut directly from VS - some kind of line ending thing)
This works and makes sense since we are expecting a comma separated list.
A few questions about the PR:
Thanks for all your help!
Cheers, Kenny
A few questions about the PR:
* Do you want the code for PageableModelFilter in the JHipsterNet.Core project (in the library)
yes i think it's better
* Or in the Configuration directory of the generated project maybe? * Do you want the changes to Startup.cs to be made when the root application is created or when the entities are applied?
i think it's simplier to do this when the root app is generated
and especially thanks to you to have detected this bug and to help me to correct it !
@nicolas63 You want me to put PageModelFilter in the JHipsterNet.Core Boot directory or Pagination directory? Since startup, Boot kinda fits. We could create a new directory? Let me know what you prefer.
Also, do you prefer I create a new branch or merge to master in my forked copy?
One more question, do you want me to update the package versions and dotnet version while I'm in there? I know we shouldn't update AutoMapper version because we are using an older mapping syntax.
@nicolas63 You want me to put PageModelFilter in the JHipsterNet.Core Boot directory or Pagination directory? Since startup, Boot kinda fits. We could create a new directory? Let me know what you prefer.
i think the right place is a new folder in https://github.com/jhipster/jhipster-net/tree/main/src/JHipsterNet.Web/Pagination
Also, do you prefer I create a new branch or merge to master in my forked copy?
as you like as long as it is on your fork 😄
One more question, do you want me to update the package versions and dotnet version while I'm in there? I know we shouldn't update AutoMapper version because we are using an older mapping syntax.
yes you can, but are you sure for automapper syntax ? we only use automapper here https://github.com/jhipster/jhipster-net/blob/930dfba8ed354bbd514c601b12642620a119ae22/src/JHipsterNet.Core/Pagination/Extensions/QueryableExtensions.cs#L19
@nicolas63 You were right about AutoMapper version - it was on 8.x which is after they made the breaking changes. (I was thinking it was 9.x, but I was wrong)
I bumped the versions from 1.0.7 to 1.0.8
I'm testing the library changes right now and should have a PR for it very soon. Then we can build and push that and I can do the PR on jhipster-dotnetcore.
@nicolas63 Ok, library PR created.
I will create the PR for jhipster-dotnetcore after this is merged!
@nicolas63 I'm going to change generator for Directory.Build.targets versions on packages to match those that are in the library. Should I update all the versions, including those not in the library to latest stable and 6.0 compatible?
While testing locally, I realized I cut and pasted some code and didn't add IOperationFilter to PageableModelFilter. I created another pull request, do I need to bump the nuget versions?
@nicolas63 I'm going to change generator for Directory.Build.targets versions on packages to match those that are in the library. Should I update all the versions, including those not in the library to latest stable and 6.0 compatible?
No just update jhipster-net
While testing locally, I realized I cut and pasted some code and didn't add IOperationFilter to PageableModelFilter. I created another pull request, do I need to bump the nuget versions?
Yes, we need an other pull request with nuget versions updated
@nicolas63 Testing... If I don't upgrade the packages in the template to the new versions that match the updated versions in the library, I get "Detected package downgrade" errors.
Also do you want me to upgrade the jhipster-dotnetcore version number from 3.4.0 to 3.4.1 (there are 30 occurrences)
@nicolas63 Ah, I see that dependabot has been creating pull requests to update package versions, so I can't update those without creating a merge conflict.
Let me know how you prefer for me to proceed and how I can help you.
@nicolas63 I created a pull request that includes the following:
Overview of the issue
I created a new application using the latest versions of jhipster and jhipster-dotnetcore. When I pull up the swagger page and try to do a get on an object, swagger returns an error: TypeError: Failed to execute 'fetch' on 'Window': Request with GET/HEAD method cannot have body.
So apparently Swagger doesn't support the parameters coming from the body. So I added the [FromQuery] modifier to the IPageable incoming object in the GetAll method in the controller:
This causes Swagger to return a nonsensical parameter list that includes all the fields on Next recursively.
Motivation for or Use Case
I expected the application to work after being generated with maybe some configuration. Perhaps I missed a step?
Reproduce the error
Suggest a Fix
I was able to get it to work by creating a class with PageNumber, PageSize and Sort in it:
I then pass that to the Get method in the controller and map that to an IPageable object manually (as a test):
JHipster Version(s)
7.8.1
JHipster configuration
JHipster configuration, a
.yo-rc.json
file generated in the root folder.yo-rc.json file
JDL for the Entity configuration(s)
entityName.json
files generated in the.jhipster
directoryJDL entity definitions
Environment and Tools
openjdk version "11.0.14.1" 2022-02-08 OpenJDK Runtime Environment Temurin-11.0.14.1+1 (build 11.0.14.1+1) OpenJDK 64-Bit Server VM Temurin-11.0.14.1+1 (build 11.0.14.1+1, mixed mode)
git version 2.33.0.windows.2
node: v16.14.2
npm: 8.7.0
Docker version 20.10.12, build e91ed57
docker-compose version 1.29.2, build 5becea4c
No change to package.json was detected. No package manager install will be executed. Congratulations, JHipster execution is complete! Sponsored with ❤️ by @oktadev.
JDL definitions
Browsers and Operating System
Edge on Windows 10