4425 add on success method to commands

[sekmiller]

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Thanks!

Related Issues

• closes Command Engine: Move indexing and other actions into "onSuccess" method #4425 move indexing to onSuccess method to be run after command completes

Pull Request Checklist

• Unit [tests][x] completed
• Integration [tests][x]: completed
• Deployment requirements: [x] None
• Documentation completed
• Merged latest from "develop" [branch][x] and resolved conflicts

[coveralls]

Coverage decreased (-0.06%) to 19.502% when pulling c117f2d on 4425-add-onSuccess-method-to-commands into 9f946f9 on develop.

[michbarsinai]

All in all looking good, and seems like the code is handling errors that it didn't handle before. Some questions:

• EJBDataverseEngine:255 - I'm not sure I was able to follow the full logic here... is this always true on the last non-inner command executed in a command sequence?
• EjbDataverseEngineInner - This class needs 1-2 lines of JavaDoc, takes time to understand why it's there.
• Index.java:228-234 - Similar lines appear in many places in the code. Maybe could be consolidated into a single method. OR - we might need an onFailure handler as well.
• Command:46 - the onSuccess methods needs some JavaDoc, Command is a class new developer refer to a lot.
• DeleteDataverseLinkingDataverseCommand:56 - new code does not re-index editedDV. Is that intentional?

This PR improves on the current design. I think it needs a bit more documentation since it's the core of the code, and people will be reading it.

[mheppler]

@sekmiller typo ("hel")?

[sekmiller]

"held". thanks