ilgrosso commented on a change in pull request #263:
URL: https://github.com/apache/syncope/pull/263#discussion_r634443411
##########
File path:
core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
##########
@@ -207,16 +207,43 @@ public void create(final Group group, final GroupTO
groupTO) {
group.setName(groupPatch.getName().getValue());
}
+ PropagationByResource<String> propByRes = new
PropagationByResource<>();
// owner
if (groupPatch.getUserOwner() != null) {
- group.setUserOwner(groupPatch.getUserOwner().getValue() == null
- ? null
- : userDAO.find(groupPatch.getUserOwner().getValue()));
+ if (StringUtils.isBlank(groupPatch.getUserOwner().getValue())) {
Review comment:
Checking for `null` equality should be enough here (rather than
`StringUtils#isBlank` since the expected value is an user key, e.g. UUID
##########
File path:
core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
##########
@@ -207,16 +207,43 @@ public void create(final Group group, final GroupTO
groupTO) {
group.setName(groupPatch.getName().getValue());
}
+ PropagationByResource<String> propByRes = new
PropagationByResource<>();
// owner
if (groupPatch.getUserOwner() != null) {
- group.setUserOwner(groupPatch.getUserOwner().getValue() == null
- ? null
- : userDAO.find(groupPatch.getUserOwner().getValue()));
+ if (StringUtils.isBlank(groupPatch.getUserOwner().getValue())) {
+ if (group.getUserOwner() != null) {
+ group.setUserOwner(null);
+ propByRes.addAll(ResourceOperation.UPDATE,
groupDAO.findAllResourceKeys(group.getKey()));
+ }
+ } else {
+ User userOwner =
userDAO.find(groupPatch.getUserOwner().getValue());
+ if (userOwner != null) {
Review comment:
Please invert this if / else check to avoid Yoda issues with checkstyle,
e.g.
```
if (userOwner == null) {
} else {
}
```
##########
File path:
core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
##########
@@ -207,16 +207,43 @@ public void create(final Group group, final GroupTO
groupTO) {
group.setName(groupPatch.getName().getValue());
}
+ PropagationByResource<String> propByRes = new
PropagationByResource<>();
// owner
if (groupPatch.getUserOwner() != null) {
- group.setUserOwner(groupPatch.getUserOwner().getValue() == null
- ? null
- : userDAO.find(groupPatch.getUserOwner().getValue()));
+ if (StringUtils.isBlank(groupPatch.getUserOwner().getValue())) {
+ if (group.getUserOwner() != null) {
+ group.setUserOwner(null);
+ propByRes.addAll(ResourceOperation.UPDATE,
groupDAO.findAllResourceKeys(group.getKey()));
+ }
+ } else {
+ User userOwner =
userDAO.find(groupPatch.getUserOwner().getValue());
+ if (userOwner != null) {
+ group.setUserOwner(userOwner);
+ propByRes.addAll(ResourceOperation.UPDATE,
groupDAO.findAllResourceKeys(group.getKey()));
+ } else {
+ LOG.debug("Unable to find user owner for group {} by key
{}",
+ group.getKey(),
groupPatch.getUserOwner().getValue() );
+ group.setUserOwner(null);
+ }
+ }
}
if (groupPatch.getGroupOwner() != null) {
- group.setGroupOwner(groupPatch.getGroupOwner().getValue() == null
- ? null
- : groupDAO.find(groupPatch.getGroupOwner().getValue()));
+ if (StringUtils.isBlank(groupPatch.getGroupOwner().getValue())) {
Review comment:
Same observations here as in the former block
##########
File path:
core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
##########
@@ -207,16 +207,43 @@ public void create(final Group group, final GroupTO
groupTO) {
group.setName(groupPatch.getName().getValue());
}
+ PropagationByResource<String> propByRes = new
PropagationByResource<>();
Review comment:
Please rename this as something like `ownerPropByRes`
##########
File path:
core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
##########
@@ -207,16 +207,43 @@ public void create(final Group group, final GroupTO
groupTO) {
group.setName(groupPatch.getName().getValue());
}
+ PropagationByResource<String> propByRes = new
PropagationByResource<>();
// owner
if (groupPatch.getUserOwner() != null) {
- group.setUserOwner(groupPatch.getUserOwner().getValue() == null
- ? null
- : userDAO.find(groupPatch.getUserOwner().getValue()));
+ if (StringUtils.isBlank(groupPatch.getUserOwner().getValue())) {
+ if (group.getUserOwner() != null) {
+ group.setUserOwner(null);
+ propByRes.addAll(ResourceOperation.UPDATE,
groupDAO.findAllResourceKeys(group.getKey()));
+ }
+ } else {
+ User userOwner =
userDAO.find(groupPatch.getUserOwner().getValue());
+ if (userOwner != null) {
+ group.setUserOwner(userOwner);
+ propByRes.addAll(ResourceOperation.UPDATE,
groupDAO.findAllResourceKeys(group.getKey()));
+ } else {
+ LOG.debug("Unable to find user owner for group {} by key
{}",
+ group.getKey(),
groupPatch.getUserOwner().getValue() );
Review comment:
remove the extra space before `);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
