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

(IndexAware)Predicate API cleanup (#13226) #15142

Conversation

@vojtechtoman
Copy link
Contributor

vojtechtoman commented Jun 10, 2019

Moved IndexAwarePredicate and VisitablePredicate to query.impl.predicates
and refactored/cleaned up the public Predicate-related API so that it does
not expose internal APIs anymore. More specifically:

  • Moved query.IndexAwarePredicate, query.VisitablePredicate,
    query.SqlPredicate/Parser, query.TruePredicate to query.impl.predicates
  • Moved query.impl.FalsePredicate and query.impl.SkipIndexPredicate to
    query.impl.predicates
  • Converted PagingPredicate and PartitionPredicate to interfaces and
    added PagingPredicateImpl and PartitionPredicateImpl to
    query.impl.predicate.
  • Converted PredicateBuilder and EntryObject to interfaces (and made
    EntryObject a nested interface in PredicateBuilder) and added
    PredicateBuilderImpl to query.impl.predicates
  • The public API classes/interfaces no longer extend IndexAwarePredicate/
    VisitablePredicate; this dependency has been moved to the impl classes.
  • Introduced new factory methods in Predicates: newPredicateBuilder(),
    sql(...), pagingPredicate(...), partitionPredicate(...)

The end result is that the public Predicate API provides only interfaces (Predicate,
PagingPredicate, and PartitionPredicate) with no dependencies on internal APIs.

EE counterpart: hazelcast/hazelcast-enterprise#3028
Fixes #13226

@vojtechtoman

This comment has been minimized.

Copy link
Contributor Author

vojtechtoman commented Jun 10, 2019

run-lab-run

@vojtechtoman vojtechtoman requested a review from taburet Jun 10, 2019
Copy link
Contributor

taburet left a comment

Looks good, just two minor comments. Exhausting work I should say.

anchorList.add(new SimpleImmutableEntry<Integer, Map.Entry<K, V>>(anchorPage, anchorEntry));
}
}
IterationType getIterationType();

This comment has been minimized.

Copy link
@taburet

taburet Jun 14, 2019

Contributor

These two (getIterationType and setIterationType) are kinda internal and provide nothing useful to a user. I think it's better to remove them from the interface and provide access from PagingPredicateAccessor only.

BTW, getAnchor also looks like a good candidate for removal, but I'm not sure about it. @jerrinot WDYT?

This comment has been minimized.

Copy link
@vojtechtoman

vojtechtoman Jun 14, 2019

Author Contributor

I was wondering about the iteraton type, too. I will take a look at it.

@@ -16,6 +16,8 @@

package com.hazelcast.query;

import com.hazelcast.query.impl.predicates.PagingPredicateImpl;

import java.util.Map;

This comment has been minimized.

Copy link
@taburet

taburet Jun 14, 2019

Contributor

I think we should add a javadoc to PagingPredicateAccessor saying something like "this class is for internal use only and may be changed without any notice".

This comment has been minimized.

Copy link
@vojtechtoman

vojtechtoman Jun 14, 2019

Author Contributor

Good idea, will do.

@vojtechtoman vojtechtoman force-pushed the vojtechtoman:fix/4.0/13226-indexawarepredicate-private branch from b9c5608 to 78286ce Jun 14, 2019
@vojtechtoman

This comment has been minimized.

Copy link
Contributor Author

vojtechtoman commented Jun 14, 2019

run-lab-run

@mmedenjak mmedenjak added this to the 4.0 milestone Jun 18, 2019
Copy link
Contributor

mmedenjak left a comment

Great stuff, congrats! Such a simple and clean solution. Added a couple of minor comments, you can take a look before merging.

Vojtech Toman added 4 commits Jun 10, 2019
Moved IndexAwarePredicate and VisitablePredicate to query.impl.predicates
and refactored/cleaned up the public Predicate-related API so that it does
not expose internal APIs anymore. More specifically:

- Moved query.IndexAwarePredicate, query.VisitablePredicate,
  query.SqlPredicate/Parser, query.TruePredicate, to query.impl.predicates
- Moved query.impl.FalsePredicate and query.impl.SkipIndexPredicate to
  query.impl.predicates
- Converted PagingPredicate and PartitionPredicate to interfaces and
  added PagingPredicateImpl and PartitionPredicateImpl to
  query.impl.predicate.
- Converted PredicateBuilder and EntryObject to interfaces (and made
  EntryObject a nested interface in PredicateBuilder) and added
  PredicateBuilderImpl to query.impl.predicates
- The public API classes/interfaces no longer extend IndexAwarepredicate/
  VisitablePredicate; this dependency has been moved to the impl classes.
- Introduced new factory methods in Predicates: newPredicateBuilder(),
  sql(...), pagingPredicate(...), partitionPredicate(...)

Fixes #13226
...and moved it to PagingPredicateAccessor.
Made it clear in the javadoc of PagingPredicateAccessor
that the class is for internal use only.
Vojtech Toman
@vojtechtoman vojtechtoman force-pushed the vojtechtoman:fix/4.0/13226-indexawarepredicate-private branch from 736f30e to c8b3f34 Jun 19, 2019
@vojtechtoman

This comment has been minimized.

Copy link
Contributor Author

vojtechtoman commented Jun 19, 2019

run-lab-run

@vojtechtoman vojtechtoman merged commit 6fabb1c into hazelcast:master Jun 19, 2019
1 check passed
1 check passed
default Test PASSed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.