Github user aledsage commented on a diff in the pull request:
    --- Diff: 
    @@ -425,6 +443,20 @@ public void update() {
    +    @Override
    +    public void changeServerPool(String groupId) {
    +        Group newGroup = (Group) 
    +        if (newGroup == null) {
    +            throw new IllegalArgumentException("Group '"+groupId+"' not 
    +        }
    +        config().set(SERVER_POOL, newGroup);
    +        if (hasServerPoolMemberTrackingPolicy()) {
    --- End diff --
    What happens is...
    If we don't already have a serverPoolMemberTrackingPolicy (e.g. because we 
haven't been started yet), then keep it that way (i.e. don't add one).
    In `hasServerPoolMemberTrackingPolicy`, it handles the case where the field 
has not been set - it is effectively transient because fields are not persisted 
(only attributes, config and adjuncts).
    When we then call `addServerPoolMemberTrackingPolicy`, it's into the code I 
didn't touch. Agreed that method is a bit weird - it'll remove it if 
`serverPoolMemberTrackerPolicy` is not null, but won't if a policy of that type 
already exists with the field being null (which presumably could happen on 
    In conclusion, I agree that `addServerPoolMemberTrackingPolicy` is a bit 
weird. But I'm ok with the logic behind the code I've changed/added :-)


Reply via email to