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

DBManager: getAll now accepts DBFilter<T>... #290

Merged
merged 1 commit into from
Nov 8, 2023
Merged

DBManager: getAll now accepts DBFilter<T>... #290

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions modules/dbsupport/src/main/java/org/jpos/ee/DBManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@ public int getItemCount(DBFilter<T>... filters) {
return totalCount.intValue();

}

public List<T> getAll(int offset, int limit, Map<String,Boolean> orders) {
return this.getAll(offset, limit, orders, null);
}

Comment on lines +84 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice PR @jrfinc, do we need this overload? And the corresponding null check in the other getAll?

Without this overload a call to getAll(offset, limit, orders) will just invoke the other with an empty array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right and we don't need it, but if removed it could break other DBManager implementations (in jPOS-EE RevisionManager for example, but could have overrides in other projects). It's an easy fix on the managers but still I thought it didn't hurt leaving it.

Regarding the null check, we need it for: Arrays.stream(filters) (I can either check for null like I did, or use Optionals. etc).

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand, I see there are two null checks, one for protecting against the nullity of the filters argument, and one for each element on it. I wonder if that is because of this call or for protection against other calls.

I was thinking that this call could be with an empty array, in case the inner null check is because of this null, but one can't be too cautious anyway right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I am thinking mainly on use cases of optional filters for a big list. It could happen that nulls get there.
I guess the devs using this call could (should?) sanitize the array and remove nulls beforehand, but it was easy to do here too.

public List<T> getAll(int offset, int limit, Map<String,Boolean> orders, DBFilter<T>... filters) {
CriteriaBuilder criteriaBuilder = db.session().getCriteriaBuilder();
CriteriaQuery<T> query = criteriaBuilder.createQuery(clazz);
Root<T> root = query.from(clazz);
Expand All @@ -93,9 +96,21 @@ public List<T> getAll(int offset, int limit, Map<String,Boolean> orders) {
orderList.add(order);
}
}
Predicate[] predicates = buildFilters(root);
if (predicates != null)
query.where(predicates);
Predicate combinedPredicate = null;
if (filters != null) {
combinedPredicate = criteriaBuilder.and(Arrays.stream(filters)
.filter(f -> f != null)
.map(f -> f.createPredicate(criteriaBuilder, root))
.toArray(Predicate[]::new));
}
Predicate[] mgrFilters = buildFilters(root);
if (mgrFilters != null) {
Predicate[] nonNullPredicates = Arrays.stream(mgrFilters).filter(f -> f != null).toArray(Predicate[]::new);
Predicate mgrPredicate = criteriaBuilder.and(nonNullPredicates);
combinedPredicate = combinedPredicate != null ? criteriaBuilder.and(mgrPredicate, combinedPredicate) : mgrPredicate;
}
if (combinedPredicate != null)
query.where(combinedPredicate);
query.select(root);
query.orderBy(orderList);
Query<T> queryImp = db.session().createQuery(query);
Expand Down
Loading