apache / apache/grails-core

AclObjectIdentityGormService::findAllByParentObjectIdAndParentAclClassName() performance issue

Open
#15,925 0 comments 0 reactions 0 assignees View on GitHub
relates-to:spring-security
Dominant language
Groovy
Stars
2.9k
Forks
975
Avg merge
1d 22h
Merged PRs (30d)
92

Description

Method AclObjectIdentityGormService::findAllByParentObjectIdAndParentAclClassName() searches now AclObjectIdentity objects so, that ALL objects of AclObjectIdentity are retrieved, and then the result is searched in this list (see below):

```
@CompileDynamic
@ReadOnly
List findAllByParentObjectIdAndParentAclClassName(Long objectId, String aclClassName) {
//findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list()
List aclObjectIdentityList = findAll()
aclObjectIdentityList.findAll { AclObjectIdentity oid ->
(oid?.parent?.aclClass?.className == aclClassName) && ( oid?.parent?.objectId == objectId)
}
}
```

This method calls AclObjectIdentityGormService::findAll(), which retrieves all AclObjectIdentity objects. Since we have about 30'000 AclObjectIdentity objects (this amount still increases), then all these objects are in hibernate session, which causes performance problems.

As a workaround, we have created our own bean inherited from AclObjectIdentityGormService, where findAllByParentObjectIdAndParentAclClassName() is overloaded and uses findQueryByParentObjectIdAndParentAclClassName() as it was earlier in the plugin before the commit 4d53a88 from 1. Dec 2017 by @sdelamo has been made.

We don't know, what was the reason of using findAll() here, but we think that such approach is algorithmically unhappy. Could you help us to solve this problem correctly, please?

Contributor guide

Open the contributing guide

Research direction

Start with AclObjectIdentityGormService::findAllByParentObjectIdAndParentAclClassName() and compare the commented findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName). Confirm the method returns the same matching AclObjectIdentity records without loading every record through findAll(); the payload names no test file to run.

Written by the indexing model from the issue text.

Assessment

Tech stack
groovy
Domain
databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.