wrenidm
wrenidm copied to clipboard
WrenSecurity
Issue #11 - React to UID Changes during ICF Update
The documentation for the ICF Update SPI very clearly states that the UID of an object can change, which is why that operation returns the updated UID. Unfortunately, though the ICF provisioner in IDM goes to great pains to ensure this updated UID gets returned to the sync operation, it appears that it gets ignored by the operation and the link object does not get updated. This issue is present in IDM 4.x and IDM 5.x (including the current release version of IDM 5.5).
This is just a band-aid patch for specifically this issue. However, the entire SyncOperation class badly needs re-factoring. The performAction() method alone is 175 lines and contains a nasty, double-nested switch statement with intentional fall-throughs.
Closes #11.
I have to admit that there are few changes that feel a bit unnecessary (especially for band-aid patch) and bloat the overall PR / diff (e.g. empty lines, line breaks, parenthesis).
I would very much like to see just the change without unnecessary line-breaks (do not break under 120 characters) and JavaDoc formatted in a consistent manner with existing JavaDocs.
Apologies for being a bit pedantic.
Happy to follow the project coding conventions, but it doesn't appear that they are posted. Given that, I'm afraid @pavelhoral that your feedback is just your personal taste at the moment. So I'm going to hold off on making additional changes to this PR until the project has posted an official convention.
@GuyPaddock as requested (and as promised in Gitter), we now have a Java style guide! Please check it out here: https://github.com/WrenSecurity/wrensec-docs/wiki/Java-Style-Guide
regarding that, some general feedback:
- please keep style changes separate from other changes in the same PR. If you're making a lot of style changes, we may ask you to move those to a separate PR so it doesn't make it difficult to review the actual change you're making.
- please avoid declaring all variables at the top of blocks; declare them as you need them. I realize that other coding styles you might be familiar with (Sun's original coding standards, for example) may have recommended declaring all variables at the top of blocks, but our style guide doesn't (and neither does Google's).
- prefer a 100 column margin (up to 120 if necessary for generics, etc), instead of an 80 column margin. this preserves the style of a lot of the existing FR code.
If you could take another pass at this PR with this feedback in mind, we would very much appreciate it.
Thanks, @Kortanul! I am at the end of a large project right now but am hoping to circle back to this in the next week or so.
