Bug Fixing Part 2

In the previous post (Bug Fixing Part 1), I discussed how to fix the bug. However, the bug is not completely fixed because if the user modifies the server configurations in the server editor (which turns isDirty flag to true), the application will NOT prompt the user to save the modifications and discard the changes.

This issue is caused in the following method, when the closeEditor is called with the flag "false", which indicates "do not save the changes":



There are a few flaws with the above fix. I will discuss these flaws in terms of various scenarios:

Scenario 1

The Fix Introduces a New Bug

1. User creates a new server and double click on the server to open the server editor:



2. The user make some modification to the server configurations in the server editor. For example, in this scenario they will change the port numbers, which will turn "isDirty" flag of the server to "true":



3. The user deletes the server. It will not matter whether the user puts a checkmark to the "delete unused server configurations" or not:



4. Since the server configurations were modified without saving, the user gets prompted to either save the changes or not:



4a. If the user selects "NO" to discard the changes, everything will be fine. The server will get deleted and removed from the servers list, and the server editor gets closed.

4b. If the user selects "Cancel", the server will get deleted and removed from the servers list, but the server editor stays opened. I argue that the existence of the "Cancel" button is unnecessary because it cannot undo the deletion (more on this matter comes shortly), as well as it will introduce the original bug: deletion of a server will not close the editor.

4c. If the user selects "YES" to save the changes, another bug is introduced: a new server with the same configuration will get created and will be added to the servers list. I believe this behavior is caused because saving the changes will update the server configuration, which in turn will trigger some method(s) in the application to update the server list and creating a new server:

Scenario 2

The Fix Has Logical Flaw

The user creates a new server, and they will make some modifications to the server configurations in the server editor (without saving them).

Then the user decides to delete the server and will get prompted with the following dialog box:



The user puts a checkmark to "delete the unused configurations" , which means they want to delete the actual files on the computer.

After clicking on OK button to confirm the deletion, the user gets prompted again to whether they want to save or discard the changes in the server editor. Although it might not be a big deal, bug we have a logical flaw in this case. If the user wants to delete the server configuration, they will not care about saving the changes. Therefore, there is no need to prompt the user again.

Reason Behind the Issues in Both Scenarios

The main reason is that the server gets deleted and removed from the servers list before the server editor gets closed. Examining the code shows the "server" instance gets de-registered and removed from the "Servers List" before closing the server editor. That's why when the there are unsaved modifications in the server editor, the application behave oddly according to the user's selection as explained in above scenarios.

Initial Fix

My initial fix was to close the server editor (and in case of unsaved modifications, asking the user to either save them or discard them) before deleting the server. After examining the code, I realized doing this fix will require modification of many codes, changing many methods signatures, and introducing strong coupling between some classes from different packages. Besides, the logical flaw that I talked about will still exist. For example, if the user decides to delete the server including the configuration files, they still gets prompted to save the modified configurations, which is un-necessary since the user intends to completely deletes the configuration files.

Proposed Solution

Introduction

The DeleteServerDialog.Java in the org.eclipse.wst.server.ui.internal package, is the class that gets called when the user tries to delete a server. This class dynamically add checkboxes in the dialog box GUI according to the server's status. For example, if the server is running, and the user tries to delete the server, two checkboxes will be added dynamically at the runtime to the GUI:



The code responsible for this dynamic creation of checkboxes is located in the protected Control createCustomArea(Composite parent) method:



The code responsible for deleting the server is located in the protected void buttonPressed(int buttonId) method:

Solution

I propose that to fix the issues with all above scenarios, dynamically add a checkbox to the delete dialog box seeking the user's decision regarding the modified configurations in the server editor. I say "dynamically" because if there is no modifications in the server editor, there is no need to display that checkbox:



Then the application can either save or discard the changes made to the configurations based on the user's choice.

To prevent the logic flaw, I recommend that this checkbox will be disabled/enabled based on the selection of "Delete unused server configuration(s)". Again, it is because if the user wants to delete the server configurations, there is no need to save them.

The main advantage of this solution is that there is no need to make several modifications in the various methods/classes. Another advantage is that we don't need to show 2 dialog boxes to the user: one of deletion confirmation and another one for saving/discarding changes.

Solution Implementation

The first step is to add a checkbox dynamically to the dialog box GUI. This can be done with the following code in the protected Control createCustomArea(Composite parent) method of DeleteServerDialog.Java:




Moreover, the following code currently exist in the protected void buttonPressed(int buttonId) method of DeleteServerDialog.Java that is responsible for deleting the server (and if checked, deleting the server configurations):



Now we can modify this code to determine if we have to either save or delete the configurations based on the user's selection of the checkboxes:

Bug Fixing Part 1 (Updated)

I believe I fixed the bug. I will confirm it with the instructor, before releasing the solution.

I debugged the Eclipse many many times and tried to follow the path (the method calls) that lead to the bug. With the aid of documentation, I could successfully find the method calls that lead to the bug. The Following steps were taken by me to find and fix the bug:

1. As mentioned in the previous post , the source of the bug is equals method in the Base.java class, which is called in the serverRemoved in the ServerEditor.java:

public void serverRemoved(IServer oldServer) {

if (oldServer.equals(server) && !isDirty())
closeEditor();
}

2. I find calling isDirty() method is unnecessary. Let me explain what this method does. When any information is modified in the server editor window, "dirty" flag of the server editor is set to "true", and isDrity() method returns the value of the flag (either true or false). This is useful since the user will get prompted to save the modified server editor before closing it.

For now, I decided to remove the call to isDirty() method completely, and I will deal with that later (Updated - Get more information in the next post "Bug Fixing Part 2"). So the modified code is the serverRemoved() method will look like this:

public void serverRemoved(IServer oldServer) {

if (oldServer.equals(server))
closeEditor();
}

3. The IF statement compares an instance of IServer interface that is passed to the method with the member variable (server) , which represents a working copy of the server. The result of this comparison is always "False". To better understand the reason behind it, let's look at the equals method in the Base.Java:



As you see, this method performs various tests to determine if the two instances are equal or not. In the case of our bug, the following line causes that this method concludes that the two instances of the server are NOT equal:

if (isWorkingCopy() != base.isWorkingCopy())
return false;

Each Server instance can have a ServerWorkingCopy instance associated with that.

isWorkingCopy() returns "False" or "True" based on whether the IServer instance is a working copy of the server (ServerWorkingCopy instance) or the actual "Server" instance.

The isWorkingCopy() of the actual Server instance always return "false", and the isWorkingCopy() of ServerWorkingCopy instance always return true.

Based on the above information, and the debugging trace, I observed that "base" object is an instance of ServerWorkingCopy and the current object "this" is the actual Server instance. Therefore, these two instances are never equal.

4. Therefore, to fix the bug I should pass the working copy of the actual "Server" instance to equals() method instead of passing the "Server" object itself.

5. Going back to serverRemoved() method: I debugged the bug path many times and examined the following files in details:

ResourceManager.java
Server.java
IServerLifeCycleList.java
GlobalCommandManager.java
IServer.java
IServerWorkingCopy.java

My goal was to determine whether the "oldServer" object that is passed to serverRemoved() object is the actual "Server" object or its working copy. I figured out that the "oldServer" is the actual "Server" object not its working copy. Therefore, if I can grab the working copy of the "Server" object, I can pass it to equals() method to fix the bug.

5. After investigation of the above java filenames and the documentation, I found out that I can get the working copy of the "Server" object from the GlobalCommandManager. Therefore, I changed the code in the serverRemoved() method to the following code:

public void serverRemoved(IServer oldServer) {
if (commandManager.getServerResource(oldServer.getId()).equals(server))
closeEditor();
}



6. commandManager.getServerResource() method gets a resource id (string) and returns the resource. In this case, I passed the id of the received server (oldServer.getId()), and the method returns a working copy of the server. Therefore, the two instances that are passed to equals() method will be equal. As the result, the server editor will be successfully closed upon deletion of the server.

7. A copy of modified ServerEditor.java can be downloaded from the following link. Just copy this file into your Eclipse Plugin workspace and replace the old file. Then, compile and run the modified code and observe that the bug is fixed.

Modified Copy of ServerEditor.java fixed by Farhad Norouzi

Assignment 2 - Bug 0.2 Release

As mentioned in the previous post (Code Finding), the code that causes the bug is in the ServerEdit.Java, in the method called serverRemoved.




I examined the body of the serverRemoved method:

public void serverRemoved(IServer oldServer) {
if (oldServer.equals(server) && !isDirty())
closeEditor();
}

The method compares the received IServer parameter (oldServer object) to the the current server object (server), as well as it calls the isDirty() method to determine if anything on the server editor has been changed. If both of these return values are true, it will close the server by calling the closeEditor() method.

I determined that the problem lies in one or more of these 3 methods:

1. equals() method of IServer .
2. isDirty() method.
3. closeEditor().

To further pintpoint the problem, I commented the if statement, so the serverRemoved method will always call closeEditor() method. I did this to find out if the closeEditor() method can successfully close the server editor:

public void serverRemoved(IServer oldServer) {
// if (oldServer.equals(server) && !isDirty())
closeEditor();
}




I ran the modified code, and tested the bug. I observed the server editor is closed successfully upon deletion of the server from the server view. Therefore, I understood that the closeEditor() does its job properly.

Then I started investigating isDity() method to make determine if it is the cause of the bug or not. Therefore, I edited the serverRemoved method like the following:

public void serverRemoved(IServer oldServer) {

if (!isDirty())
closeEditor();
}




I ran the modified code, and observed that the server editor is successfully closed upon deletion of the server from the server view IF AND ONLY IF the server editor is not modified. Otherwise, it will stay open. Therefore, I understood that isDirty method is not the cause of the bug, but more code needs to be added in the serverRemoved method to ask the user to save the changes and close the server editor.

My next step was testing equals() method. Therefore, I edited the code in the removedServer same as following:

public void serverRemoved(IServer oldServer) {

if (oldServer.equals(server))
closeEditor();
}




I ran the modified code, and observed that the server editor is not closed upon deletion of the server. Therefore, I found out the main root of the bug is equals() method.

Conclusion:

Based on the above investigation it is determined the main cause of the bug is equals() method that is called in serverRemoved method.

To fix the bug, one has to fix the equals() method and add more code in serverRemoved method to prompt the user to save the server editor if isDirty method returns true (which means the user has modified the server editor).

Code Finding Procedure in Eclipse WTP for Bug# 240167

I was able to locate the code for the bug# 240167 based on the demonstration by Jordan in the class, as well as, debugging and using breakpoints in org.eclipse.wst.server.ui -> serverui -> org.eclipse.wst.server.ui.internal.editor package .

Since the bug is about the application not closing the server editor when deleting the server, I started investigating the org.eclipse.wst.server.ui.internal.editor package. I put several breakpoints in the java files (such as ServerEditor.Java and ServerEditorCore.Java) that I suspect the code related to bug might exist.

Then I started debugging the code. Upon deletion of the server in the target platform (Eclipse), I noticed the Eclipse Plug-In is stopped in one of my breakpoints in ServerEditor.Java. I started using debugger feature of "Step-Into" to step into the code, and I noticed when the user tries to delete the server, the following method in ServerEditor.Java is called :

public void serverRemoved(IServer oldServer);




I could successfully located the code that causes the bug. In the next post, I'll explain how to start fixing the bug.

Project Deliverable Version 0.1 - Reproduce Bug# 240167, Normal, P3

I could successfully be able to reproduce the Bug# 240167 and observed that the deleting a Tomcat 5.5 server from the server view will NOT close the server editor.

Bug Description

When the user deletes a Tomcat 5.5 server from the server view, the editor will not be closed.

Bug Cause

The editor will never be closed when the server is deleted, because the method equals() in Base.java returns always false.

The method serverRemoved() from ServerEditor.java invoks equals() for IServer and IServerWorkingCopy objects. While in ServerWorkingCopy.java the method isWorkingCopy() returns always true in Server.java isWorkingCopy()returns always false.

That is why equals() in Base returns false and serverRemoved() cannot reach closeEditor() [Jordan Anastasiade].

Possible Solution

Fix the method equals() in Base to return the correct value [Jordan Anastasiade].

Steps to Reproduce the Bug

Following steps demonstrate how to reproduce the Bug# 240167:

Step 1

• Open Eclipse WTP
  


• Create a new Tomcat 5.5 server by selecting New from the File menu (File > New > Other).
  


• The New Wizard is opened. Type in server in the textbox and select Server. Click on Next.
  


• Select Tomcat v5.5 server (Apache > Tomcat v5.5 Server). Click on Next.
  


• Click on Browse to select Tomcat 5.5 server installation location.
  
  
  If you do not have an installed Tomcat 5.5 server on your computer, click on Download and Install button to download and install the server. Click on Next.
  


• Click on Finish. The wizard is closed and the server is added to the Servers view.
  

Step 2

Double click on Tomcat v5.5 Server at localhost from the Servers view to open the server editor.

Step 3

Right click on Tomcat v5.5 Server at localhost from the Servers view and select Delete to delete the server from the Servers view.

Click on OK button on the Delete Server dialog box (it will NOT make a difference whether you put a check-mark to Delete unused server configuration(s) or not).

Observe that the server is deleted from the Servers view, but the server editor is still open.

Reactions to "Cathedral and Bazaar" and "Revolution OS"

His motivation for coding started from cathedrals. He grew up with the early video game consoles. He thought the games were very good and they always had something new to discover. Then, he decided to create the best cathedral in the world and impress many people.

I enrolled in DPS909 course as my first step into "Open Source" realm. After reading Eric Raymond's "The Cathedral and the Bazaar", I believe I have understood what the “Open Source” means, and it’s inspired me. Open source software is similar to a bazaar where the programmers create most of its existence.

Open Source is a well-organized method to gather programmers without organization overhead. It is also efficient to make sure that the produced software will satisfy the public. The Bazaar model has produced a solid operating system (Linux) and a popular browser (Firefox), which convinces me that the Bazaar model truly works.