Alexander Wels has posted comments on this change.
Change subject: userportal,webadmin: frontend refactor phase 2
......................................................................
Patch Set 4:
(8 comments)
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java
Line 85:
Line 86: /**
Line 87: * The query start event, legacy.
Line 88: */
Line 89: Event queryStartedEvent = new Event(queryStartedEventDefinition);
If they are final I can't replace them in the unit tests with mocked versions.
These are legacy events that I am looking to replace in phase 3.
Line 90:
Line 91: /**
Line 92: * The query complete event definition, legacy.
Line 93: */
Line 96: /**
Line 97: * The query complete event, legacy.
Line 98: */
Line 99: Event queryCompleteEvent = new Event(queryCompleteEventDefinition);
Line 100:
If they are final I can't replace them in the unit tests with mocked versions.
These are legacy events that I am looking to replace in phase 3.
Line 101: /**
Line 102: * The {@code frontendFailureEvent} event.
Line 103: */
Line 104: Event frontendFailureEvent = new Event("FrontendFailure",
Frontend.class); //$NON-NLS-1$
Line 100:
Line 101: /**
Line 102: * The {@code frontendFailureEvent} event.
Line 103: */
Line 104: Event frontendFailureEvent = new Event("FrontendFailure",
Frontend.class); //$NON-NLS-1$
If they are final I can't replace them in the unit tests with mocked versions.
These are legacy events that I am looking to replace in phase 3.
Line 105:
Line 106: /**
Line 107: * The {@code frontendNotLoggedInEvent} event.
Line 108: */
Line 105:
Line 106: /**
Line 107: * The {@code frontendNotLoggedInEvent} event.
Line 108: */
Line 109: Event frontendNotLoggedInEvent = new Event("NotLoggedIn",
Frontend.class); //$NON-NLS-1$
If they are final I can't replace them in the unit tests with mocked versions.
These are legacy events that I am looking to replace in phase 3.
Line 110:
Line 111: /**
Line 112: * The context the current operation is run against.
Line 113: */
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/OperationProcessor.java
Line 133: * @param manager The {@code VdcOperationManager} that has the
operation.
Line 134: * @return A VdcOperationCallback.
Line 135: */
Line 136: @SuppressWarnings("unchecked")
Line 137: private VdcOperationCallback<?, ?> createCallback(final
VdcOperationManager manager) {
renamed 'newCallback' to replacementCallback. I didn't feel the wrapper moniker
was appropriate as we are not really wrapping the callbacks, we are replacing
the callbacks.
Line 138: return new VdcOperationCallback<VdcOperation<?, ?>, Object>()
{
Line 139: @Override
Line 140: public void onSuccess(final VdcOperation<?, ?> operation,
final Object result) {
Line 141: // Do nothing more than call original callback.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperation.java
Line 9: * @param <T> The type of operation.
Line 10: * @param <P> The parameter type for the operation.
Line 11: */
Line 12: @SuppressWarnings("rawtypes")
Line 13: public class VdcOperation<T, P> implements Serializable {
final classes are evil, evil, evil. They make impossible to properly unit test.
So no final classes for me.
Line 14: /**
Line 15: * Generated serial version UID.
Line 16: */
Line 17: private static final long serialVersionUID = -6569717023385458462L;
Line 178: @SuppressWarnings("unchecked")
Line 179: VdcOperation<T, P> otherOperation = (VdcOperation<T, P>)
other;
Line 180: result = operationType.equals(otherOperation.getOperation())
Line 181: &&
parameter.equals(otherOperation.getParameter());
Line 182: return result;
That is exactly the behavior I am going for. Two identical queries with
different callbacks should match the same, that way I can ignore one of them as
the other query is already doing the job.
In the interest of being consistent, I can take the callback out of the
hashcode.
Line 183: }
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java
Line 64: // If the user is logged in, or the user is not logged in and
the operation does not allow duplicates (aka
Line 65: // it is a query, and not an action). And the operation is not
already in the queue || the operation is
Line 66: // an action (allows duplicates). Then add this operation to
the queue, and process the queue immediately.
Line 67: if ((loggedIn || isPublic) &&
(!operationQueue.contains(operation)
Line 68: || operation.allowDuplicates()) &&
operationQueue.add(operation)) {
Good point, the expression is relatively complex. Also good point about calling
the onFailure of the callback if the user is not allowed to execute the code.
Line 69: processor.processOperation(this);
Line 70: }
Line 71: }
Line 72:
--
To view, visit http://gerrit.ovirt.org/17356
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id19491b8fd4f30ad3a88790eaad664d679b35e22
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
