Community
Participate
Working Groups
Build Identifier: Server.setModuleStatus(IModule[], IStatus) and Server.setStatus(IStatus) need to fire a change event so that the ServersView2 can react to the status change and update the status label accordingly Reproducible: Always
Created attachment 203295 [details] Patch v1.0 Overview of changes: org.eclipse.wst.server.core.ServerEvent New APIs for handling IStatus as well as a new STATUS_CHANGE constant org.eclipse.wst.server.core.internal.Server Introduce fireServerStatusChangeEvent and fireModuleStatusChangeEvent, called in setServerStatus and setModuleStatus methods respectively org.eclipse.wst.server.ui.internal.cnf.ServersView2.refreshServerContent(IServer) Deal with new events from statuses that would require a refresh of the server content or server state Testing done: All tests were done with a server adapter that will output state and status for the server and modules. 1. Server status change - ensure fireServerStatusChangeEvent method called in Server.java and refreshServerState method called in ServersView2 (to refresh the view) 2. Module status change - ensure fireModuleStatusChangeEvent method called in Server.java and refreshServerContent method called in ServersView2 (to refresh the view) 3. Tested existing server state and module state updates - ensure the view still updates correctly during server operations (start, restart, stop) and module actions (publish, start, stop, restart)
A couple of comments: 1. How does the status check event pass the org.eclipse.wst.server.core.ServerEvent.checkKind() at the end of the constructor of org.eclipse.wst.server.core.ServerEvent.ServerEvent(int, IServer, IModule[], int, int, boolean, IStatus). Does checkKind() need to be modified to accept STATUS_CHANGE as well? 2. Update copyright statements as needed. Other than that, the logic looks good and testing is sufficient.
Also, I recommend to do a regression test to make sure the existing server change event will cause the server view to refresh.
Created attachment 203362 [details] Patch v1.1 Elson, regarding your comments: 1. The reason why org.eclipse.wst.server.core.ServerEvent.checkKind() had worked without adding the new STATUS_CHANGE was due to the existing logic in checkKind(). It checks for whether if (i != 0 && i != 1) or will return an IllegalArgumentException. In the cases where STATUS_CHANGE was used, it would return 0. I have added STATUS_CHANGE to this method, as something that will increment the "i" value. There are 10 places where a ServerEvent is created, that would create different "kind" values. The output below shows the "kind" values and what the resulting "i" value would be: kind=34 i=1 kind=36 i=1 kind=33 i=1 kind=40 i=1 kind=18 i=1 kind=34 i=1 kind=20 i=1 kind=16 i=0 kind=17 i=1 kind=24 i=1 The output ensures that the new increment will not cause an IllegalArgumentException for any of the existing ServerEvent creations (all are in the Server's fire____ methods). The reason why i=0 is valid is due to the method fireServerChangeEvent, which will pass in the value ServerEvent.SERVER_CHANGE as the kind (no bitwise OR modifies its value) 2. The copyright headers were already updated to 2011, these files were previously modified in 2011 3. I have done regression testing to ensure that existing events called refreshServerState and refreshServerState (i.e. ServerEvents that were not STATUS_CHANGE, such as server and module state changes)
(In reply to comment #4) > Created attachment 203362 [details] > Patch v1.1 > > Elson, regarding your comments: > > 1. The reason why org.eclipse.wst.server.core.ServerEvent.checkKind() had > worked without adding the new STATUS_CHANGE was due to the existing logic in > checkKind(). > > It checks for whether if (i != 0 && i != 1) or will return an > IllegalArgumentException. In the cases where STATUS_CHANGE was used, it would > return 0. > > I have added STATUS_CHANGE to this method, as something that will increment the > "i" value. There are 10 places where a ServerEvent is created, that would > create different "kind" values. The output below shows the "kind" values and > what the resulting "i" value would be: > > kind=34 i=1 > kind=36 i=1 > kind=33 i=1 > kind=40 i=1 > kind=18 i=1 > kind=34 i=1 > kind=20 i=1 > kind=16 i=0 > kind=17 i=1 > kind=24 i=1 > > The output ensures that the new increment will not cause an > IllegalArgumentException for any of the existing ServerEvent creations (all are > in the Server's fire____ methods). > > The reason why i=0 is valid is due to the method fireServerChangeEvent, which > will pass in the value ServerEvent.SERVER_CHANGE as the kind (no bitwise OR > modifies its value) > > 2. The copyright headers were already updated to 2011, these files were > previously modified in 2011 > > 3. I have done regression testing to ensure that existing events called > refreshServerState and refreshServerState (i.e. ServerEvents that were not > STATUS_CHANGE, such as server and module state changes) I forgot to mention, I had done the same tests as mentioned in comment #1 and also additional breakpoint tests as described in point #3 in comment #4
I updated the copyright statement for ServerEvent on 32M code stream.
Code released to 32M and HEAD
Code released to 33M
New Gerrit change created: https://git.eclipse.org/r/109070
