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

HSEARCH-3591 Expect a function as parameter for SearchQueryContext#sort #2008

Merged
merged 7 commits into from Jun 7, 2019

Conversation

yrodiere
Copy link
Member

@yrodiere yrodiere commented Jun 6, 2019

https://hibernate.atlassian.net//browse/HSEARCH-3591

Please review commit by commit, as commit messages explain why I do what I do.

@coveralls
Copy link

coveralls commented Jun 6, 2019

Pull Request Test Coverage Report for Build 3

  • 93 of 96 (96.88%) changed or added relevant lines in 23 files are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 89.181%

Changes Missing Coverage Covered Lines Changed/Added Lines %
engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/CompositeSortContextImpl.java 6 7 85.71%
engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/DelegatingSearchSortFactoryContext.java 1 3 33.33%
Files with Coverage Reduction New Missed Lines %
engine/src/main/java/org/hibernate/search/engine/common/dsl/spi/DslExtensionState.java 7 78.79%
Totals Coverage Status
Change from base Build 275: -0.05%
Covered Lines: 18414
Relevant Lines: 20648

💛 - Coveralls

@fax4ever fax4ever self-assigned this Jun 6, 2019
Copy link
Contributor

@fax4ever fax4ever left a comment

Choose a reason for hiding this comment

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

Nice job. +1 to merge. Of course after the rebase

@@ -19,11 +19,11 @@
DefaultSearchQueryContext<H, C>,
H,
SearchPredicateFactoryContext,
SearchSortContainerContext,
SearchSortFactoryContext,
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems not aligned

For consistency with SearchQueryResultContext#predicate(Function<? super PDC,SearchPredicateTerminalContext>)
and SearchQueryResultDefinitionContext#asProjection(Function<? super PJC,? extends SearchProjectionTerminalContext<P>>).
…eryResultContext.predicate(Function)

For consistency with all the other, similar methods, such as
NestedPredicateFieldContext#nest(Function<? super SearchPredicateFactoryContext,? extends SearchPredicateTerminalContext>).
…nal DSL state in SearchQueryContext#sort(Function)

This is the first step to making the *structure* of sorts immutable in
the Sort DSL.
Now that we don't rely on getResultingBuilders() to contribute sorts to
the query, we can remove a lot of complexity, in particular the concept
of SearchSortContributor which isn't necessary anymore.

This also brings the sort DSL closer to the predicate and projection
DSLs.
…ntext

For consistency with the other DSLs, and because this name is now more
fitting as the "factory context" is no longer mutable (you can't add
sorts to it, you can just create other contexts that are based on it).
…Function) in tests

Since we renamed the type from container to factory, that makes more
sense. Plus, it's consistent with what we do for projections and
predicates.
@yrodiere
Copy link
Member Author

yrodiere commented Jun 7, 2019

Thanks. I addressed your comment and rebased. Waiting for CI.

…building composite sorts

See in particular IndexSearchDocumentRepositoryImpl#searchAroundMe to
get an idea of why we need this.
@yrodiere yrodiere merged commit 3c7d7a6 into hibernate:master Jun 7, 2019
@yrodiere
Copy link
Member Author

yrodiere commented Jun 7, 2019

Merged, thanks!

@yrodiere yrodiere deleted the HSEARCH-3591 branch June 21, 2019 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants