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.

Sign up for GitHub

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

Jump to bottom

DATACMNS-1785 - Add QuerydslWebFluxConfiguration and QuerydslPredicateArgumentResolver #2274

Closed
wants to merge 1 commit into from
Closed

DATACMNS-1785 - Add QuerydslWebFluxConfiguration and QuerydslPredicateArgumentResolver #2274

wants to merge 1 commit into from

Conversation

matiasah
Copy link
Contributor

This pull request partially solves #2200 Adds QuerydslWebFluxConfiguration and QuerydslPredicateArgumentResolver for WebFlux.

It's necessary to solve #1846 to completely close #2200.

@pivotal-issuemaster
Copy link

@matiasah Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@matiasah Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 16, 2021
@matiasah matiasah changed the title Partially solve #2200 DATACMNS-1785 - Add QuerydslWebFluxConfiguration and QuerydslPredicateArgumentResolver Jan 16, 2021
@mp911de mp911de self-assigned this Jan 18, 2021
mp911de
mp911de requested changes Jan 18, 2021
View reviewed changes
Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. Please make sure to use tab indentation (see our formatting settings at https://github.com/spring-projects/spring-data-build/blob/master/etc/ide/eclipse-formatting.xml) and consider comments from #2200.

@matiasah
Copy link
Contributor Author

I applied the formatting settings on Visual Studio Code RedHat Java Plugin with the eclipse formatting (the link you commented on raw form), and auto-indented the files I modified. Is it good enough or should I install Eclipse to do that?

@mp911de mp911de mentioned this pull request Jan 19, 2021
Closed
@mp911de
Copy link
Member

mp911de commented Jan 19, 2021

Using a plugin that follows Eclipse formatting is perfectly fine, no need to use Eclipse. From a quick look, the code style looks fine now.

mp911de
mp911de reviewed Jan 19, 2021
View reviewed changes
Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

Thanks a lot. The changes look pretty good. I left few comments addressing very minor issues. Care to have a look and squash your commits once done? Feel free to force-push and leave a comment once we should have another look.

src/main/java/org/springframework/data/web/config/ReactiveQuerydslWebConfiguration.java Outdated
* Querydsl-specific web configuration for Spring Data. Registers a {@link HandlerMethodArgumentResolver} that builds up
* {@link Predicate}s from web requests.
*
* @author Oliver Gierke
Copy link
Member

Choose a reason for hiding this comment

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

Nit: That's a new class. No need to copy over the original authors, having just you as @author is sufficient.

src/main/java/org/springframework/data/web/config/ReactiveQuerydslWebConfiguration.java Outdated

/**
* Default {@link ReactiveQuerydslPredicateArgumentResolver} to create Querydsl {@link Predicate} instances for
* Spring MVC controller methods.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Spring WebFlux (instead MVC).

...in/java/org/springframework/data/web/querydsl/ReactiveQuerydslPredicateArgumentResolver.java Outdated
public ReactiveQuerydslPredicateArgumentResolver(QuerydslBindingsFactory factory,
Optional<ConversionService> conversionService) {
super(factory, conversionService);
// TODO Auto-generated constructor stub
Copy link
Member

Choose a reason for hiding this comment

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

Nit: There are a few TODO comments that should be removed.

...in/java/org/springframework/data/web/querydsl/ReactiveQuerydslPredicateArgumentResolver.java Outdated
* {@link HandlerMethodArgumentResolver} to allow injection of {@link com.querydsl.core.types.Predicate} into Spring
* WebFlux controller methods.
*
* @author Christoph Strobl
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You're the original author, so no need to copy over the original authors.

@matiasah
Copy link
Contributor Author

I'm not sure if I squashed the commits correctly.

Squashed commit of the following:
aaf263d 
commit cea8d79
Merge: 1e5a890 1a25c12
Author: Ubuntu <Ubuntu>
Date:   Tue Jan 19 22:21:51 2021 +0000

    Merge branch 'webflux-1' of https://github.com/matiasah/spring-data-commons into webflux-1

commit 1e5a890
Author: Ubuntu <Ubuntu>
Date:   Tue Jan 19 22:21:44 2021 +0000

    Squashed commit of the following:

    commit 1a25c12
    Author: Ubuntu <Ubuntu>
    Date:   Tue Jan 19 22:04:18 2021 +0000

        Correcting some comments

    commit e4c0c01
    Author: Ubuntu <Ubuntu>
    Date:   Tue Jan 19 00:08:23 2021 +0000

        Add ReactiveQuerydslPredicateArgumentResolver and ReactiveQuerydslWebConfiguration

    commit eacf835
    Author: Ubuntu <Ubuntu>
    Date:   Sat Jan 16 00:01:23 2021 +0000

        Revert code format

    commit 1069af3
    Author: Ubuntu <Ubuntu>
    Date:   Fri Jan 15 23:53:08 2021 +0000

        Adding missing @author annotation

    commit c778258
    Author: Ubuntu <Ubuntu>
    Date:   Fri Jan 15 23:44:29 2021 +0000

        Add package-info to WebFlux folders

    commit 45711bb
    Author: Ubuntu <Ubuntu>
    Date:   Fri Jan 15 23:41:48 2021 +0000

        Add QuerydslWebFluxConfiguration

    commit 5c343b5
    Author: Ubuntu <Ubuntu>
    Date:   Fri Jan 15 23:40:47 2021 +0000

        Add QuerydslPredicateArgumentResolver for WebFlux

commit 1a25c12
Author: Ubuntu <Ubuntu>
Date:   Tue Jan 19 22:04:18 2021 +0000

    Correcting some comments

commit e4c0c01
Author: Ubuntu <Ubuntu>
Date:   Tue Jan 19 00:08:23 2021 +0000

    Add ReactiveQuerydslPredicateArgumentResolver and ReactiveQuerydslWebConfiguration

commit eacf835
Author: Ubuntu <Ubuntu>
Date:   Sat Jan 16 00:01:23 2021 +0000

    Revert code format

commit 1069af3
Author: Ubuntu <Ubuntu>
Date:   Fri Jan 15 23:53:08 2021 +0000

    Adding missing @author annotation

commit c778258
Author: Ubuntu <Ubuntu>
Date:   Fri Jan 15 23:44:29 2021 +0000

    Add package-info to WebFlux folders

commit 45711bb
Author: Ubuntu <Ubuntu>
Date:   Fri Jan 15 23:41:48 2021 +0000

    Add QuerydslWebFluxConfiguration

commit 5c343b5
Author: Ubuntu <Ubuntu>
Date:   Fri Jan 15 23:40:47 2021 +0000

    Add QuerydslPredicateArgumentResolver for WebFlux
@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 20, 2021
@mp911de mp911de added this to the 2.5 M3 (2021.0.0) milestone Jan 25, 2021
mp911de pushed a commit that referenced this pull request Jan 25, 2021
@matiasah @mp911de
DATACMNS-1785 - Add ReactiveQuerydslPredicateArgumentResolver and Rea…
a839690 
…ctiveQuerydslWebConfiguration.

We now provide ReactiveQuerydslPredicateArgumentResolver to resolve Querydsl Predicates when using Spring WebFlux.

Related ticket: #2200.
Original pull request: #2274.
mp911de added a commit that referenced this pull request Jan 25, 2021
@mp911de
DATACMNS-1785 - Polishing.
d57fb3e 
Introduce constructors accepting non-null ConversionService, handle conversion service defaulting in configuration classes by using ObjectProvider. Extract common code to create a predicate in getPredicate(…) method. Add unit test.

Convert spaces to tabs.

Related ticket: #2200.
Original pull request: #2274.
@mp911de
Copy link
Member

mp911de commented Jan 25, 2021

Thank you for your contribution. That's merged and polished now.

On a related note: Make sure to configure your local Git client with a proper name and mail address as the author of your commits is Ubuntu <Ubuntu>. We fixed that in the course of the merge.

@mp911de mp911de closed this Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mp911de mp911de mp911de requested changes

Assignees

@mp911de mp911de

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
2.5 M3 (2021.0.0)
Development

Successfully merging this pull request may close these issues.

Support for Querydsl predicate binding on WebFlux [DATACMNS-1785]
4 participants
@matiasah @pivotal-issuemaster @mp911de @spring-projects-issues