Remove reliance on bidirectional association in task-user relation #6802
Conversation
BartChris
changed the title
BartChris
changed the title
| * @param user the processing user | ||
| * @return list of tasks in progress for the given user | ||
| */ | ||
| public List<Task> getTasksInProgress(User user) { |
There was a problem hiding this comment.
Just a question: Would an additional restriction to the used client of the current user not even more helpful? If an user is assigned to more clients than this list here would include all the tasks over all clients and not only the tasks of the current used client in the UI.
There was a problem hiding this comment.
Yes, might be useful.
There was a problem hiding this comment.
We have not filtered by client before, maybe this is not necessary, as this method is only called in special places.
| * @return list of tasks in progress for the given user | ||
| */ | ||
| public List<Task> getTasksInProgress(User user) { | ||
| String hql = "FROM Task t WHERE t.processingUser = :user " |
There was a problem hiding this comment.
Limiting / restrict the task list to tasks where the user is set as processing user is may to restrictive as not necessary the user who need this list as worked on this task. I did not even see such restriction in the former place where only assigned processes to the task (non null) and task state in progress the restrictions are.
There was a problem hiding this comment.
Good point, i have to think about that.
There was a problem hiding this comment.
I just noticed that this method is mostly used for emptiness checks; we can probably optimize: If only existence is checked, we do not need to retrieve all the tasks and should do an existence check.
There was a problem hiding this comment.
I did not even see such restriction in the former place
return user.getProcessingTasks().stream()I think establishing the stream in that way also limited the tasks down to those the user is actually assigned to before.
BartChris
force-pushed
the
task-processing-user-refactor
branch
from
f7925b8 to
4abe94b
Compare
|
I refactored more to make it more efficient. |
BartChris
force-pushed
the
task-processing-user-refactor
branch
from
4abe94b to
d8f8aab
Compare
| <h:panelGroup styleClass="action" | ||
| rendered="#{SecurityAccessController.hasAuthorityToUnassignTasks()}" | ||
| title="#{empty UserForm.getTasksInProgress(item) ? msgs.userWithoutTasks : msgs.unassignTasks}"> | ||
| title="#{UserForm.hasTasksInProgress(item) ? msgs.unassignTasks : msgs.userWithoutTasks}"> |
There was a problem hiding this comment.
It could be that the user list also benefits from the more efficient checks.
BartChris
changed the title
|
Should be rebased on #6803 |
2 participants
Adresses #5813 (But no full fix of the described memory usage since this probably requires more changes)
As outlined in #5813 (comment) bidrectional access to the non owning side of a relation can be problematic if the collections are too large. I would prefer if we remove the collection property.
to not encourage access to and hydration of potential large collections. But we use those accessors in tests so i only replaced the application-level usage of it with an HQL query. Since the
processingTasksaccessor is no longer used at runtime, we do not need to maintain manual synchronization between both sides of the association.