chore: use common function for building query (#31018)

The query execution methods, `queryAllExecute`, `queryOneExecute` etc.,
aren't using the same function to build the `Query` object. This PR
fixes it by using the common function.

But the common function has a problem. Check this out, this is the
current implementation:

```java
Query query = new Query();
criterias.stream().forEach(criteria -> query.addCriteria(criteria));
if (aclPermission == null) {
    query.addCriteria(new Criteria().andOperator(notDeleted()));
} else {
    query.addCriteria(new Criteria().andOperator(notDeleted(), userAcl(permissionGroups, aclPermission)));
}

if (!isEmpty(projectionFieldNames)) {
    projectionFieldNames.stream().forEach(fieldName -> query.fields().include(fieldName));
}

return query;
```

Here, we use `addCriteria` to add each of the criteria items given to
us, into the `query`. After that, we use `.andOperator` to add the
not-deleted and permission checks.

Looks good on the surface. Let's take an example. If the given criteria
list has `fieldName = "abc"` as the condition, this will end up in the
final query as (pseudo-code representation):

```javascript
{
	fieldName: "abc",
	$and: {
		$and: {
			deleted: false or missing,
			deletedAt: null
		},
		policies: {
			$elemMatch: permission check here,
		}
	}
}
```

Perfectly working query. Now, what if the incoming criteria list is a
little more complex, and has an `or` condition in it. This is what we
end up with:

```javascript
{
	$or: {
		field1: "val",
		field2: "val"
	},
	$and: {
		$and: {
			deleted: false or missing,
			deletedAt: null
		},
		policies: {
			$elemMatch: permission check here,
		}
	}
}
```

We end up with a `$or` and `$and` next to each other. This doesn't make
sense to MongoDB.

The way the query is built in the `queryAllExecute` method previously,
actually doesn't fall into this. That's the version we're changing the
common method into now. This is what it looks like:

```java
final ArrayList<Criteria> criteriaList = new ArrayList<>(criterias);
criteriaList.add(notDeleted());

final Criteria permissionCriteria = userAcl(permissionGroups, aclPermission);
if (permissionCriteria != null) {
    criteriaList.add(permissionCriteria);
}

final Query query = new Query(new Criteria().andOperator(criteriaList.toArray(new Criteria[0])));

if (!isEmpty(projectionFieldNames)) {
    query.fields().include(projectionFieldNames.toArray(new String[0]));
}

return query;
```

With this, the resulting query looks something like this:

```javascript
{
	$and: {
		$or: {
			field1: "val",
			field2: "val"
		},
		$and: {
			deleted: false or missing,
			deletedAt: null
		},
		policies: {
			$elemMatch: permission check here,
		}
	}
}
```

This isn't new code. This is how we've been building the query for
`queryAll` today. By moving this to the common method, we have this
resilient query building for `queryOne` and `queryFirst` as well.
This commit is contained in:
Shrikant Sharat Kandula 2024-02-12 12:58:42 +05:30 committed by GitHub
parent 2147b9fcea
commit 632c8a7b69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 11 additions and 53 deletions

View File

@ -5,10 +5,7 @@ import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.repositories.ce.params.QueryAllParams;
import com.mongodb.bulk.BulkWriteResult;
import com.mongodb.client.result.InsertManyResult;
import org.springframework.data.domain.Sort;
import org.springframework.data.mongodb.core.query.Criteria;
import org.springframework.data.mongodb.core.query.Update;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import java.util.List;
@ -25,14 +22,6 @@ public interface AppsmithRepository<T extends BaseDomain> {
QueryAllParams<T> queryBuilder();
Flux<T> queryAllWithStrictPermissionGroups(
List<Criteria> criterias,
Optional<List<String>> includeFields,
Optional<AclPermission> permission,
Sort sort,
int limit,
int skip);
Mono<T> setUserPermissionsInObject(T obj, Set<String> permissionGroups);
Mono<T> setUserPermissionsInObject(T obj);

View File

@ -313,62 +313,31 @@ public abstract class BaseAppsmithRepositoryCEImpl<T extends BaseDomain> {
List<String> projectionFieldNames,
Set<String> permissionGroups,
AclPermission aclPermission) {
Query query = new Query();
criterias.stream().forEach(criteria -> query.addCriteria(criteria));
if (aclPermission == null) {
query.addCriteria(new Criteria().andOperator(notDeleted()));
} else {
query.addCriteria(new Criteria().andOperator(notDeleted(), userAcl(permissionGroups, aclPermission)));
final ArrayList<Criteria> criteriaList = new ArrayList<>(criterias);
criteriaList.add(notDeleted());
final Criteria permissionCriteria = userAcl(permissionGroups, aclPermission);
if (permissionCriteria != null) {
criteriaList.add(permissionCriteria);
}
final Query query = new Query(new Criteria().andOperator(criteriaList.toArray(new Criteria[0])));
if (!isEmpty(projectionFieldNames)) {
projectionFieldNames.stream().forEach(fieldName -> query.fields().include(fieldName));
query.fields().include(projectionFieldNames.toArray(new String[0]));
}
return query;
}
public Flux<T> queryAllWithStrictPermissionGroups(
List<Criteria> criterias,
Optional<List<String>> includeFields,
Optional<AclPermission> permission,
Sort sort,
int limit,
int skip) {
Mono<Set<String>> permissionGroupsMono = ReactiveSecurityContextHolder.getContext()
.map(ctx -> ctx.getAuthentication())
.map(auth -> auth.getPrincipal())
.flatMap(principal -> getStrictPermissionGroupsForUser((User) principal));
return permissionGroupsMono.flatMapMany(permissionGroups -> queryBuilder()
.criteria(criterias)
.fields(includeFields.orElse(null))
.permission(permission.orElse(null))
.permissionGroups(permissionGroups)
.sort(sort)
.limit(limit)
.skip(skip)
.all());
}
public QueryAllParams<T> queryBuilder() {
return new QueryAllParams<>(this);
}
public Flux<T> queryAllExecute(QueryAllParams<T> params) {
return tryGetPermissionGroups(params).flatMapMany(permissionGroups -> {
final ArrayList<Criteria> criteriaList = new ArrayList<>(params.getCriteria());
criteriaList.add(notDeleted());
final Criteria permissionCriteria = userAcl(permissionGroups, params.getPermission());
if (permissionCriteria != null) {
criteriaList.add(permissionCriteria);
}
final Query query = new Query(new Criteria().andOperator(criteriaList.toArray(new Criteria[0])));
if (!CollectionUtils.isEmpty(params.getFields())) {
query.fields().include(params.getFields().toArray(new String[0]));
}
final Query query = createQueryWithPermission(
params.getCriteria(), params.getFields(), permissionGroups, params.getPermission());
if (params.getSkip() > NO_SKIP) {
query.skip(params.getSkip());