[jboss-dev-forums] [Design of JBoss Collaboration Server] - Minor Bug in MailboxServiceImpl.java?

[sappenin]

Hey All,

Not sure if this is a bug or not, but it seems to be.  If forum feedback concurs, I'll create a patch.  

In org.jboss.mail.mailbox.MailboxServiceImpl.moveFolder(...) [line 824], there appears to be a minor typo with the moveFolder function.

Below is the current version of the function.  'folder' is the folder to be moved, 'target' is the new parent folder, and name is the name of the folder.  The function loads 'folder' by id, then loads the target folder by id.    It then (assuming the folder and target aren't the same) removes 'folder' from its parent.  Next, it appers to add the target folder (i.e., the new parent) to itself via the call "t.addFolder(t)" ---> I think this is the bug.  


  | @Tx(TxType.REQUIRED)
  | 	public Folder moveFolder(Folder folder, Folder target, String name) {
  | 		Folder f = session.find(Folder.class, folder.getId());
  | 		Folder t = session.find(Folder.class, target.getId());
  | 		f.setName(name);
  |         if(folder.getId() != t.getId()) {
  |             f.getParent().getFolders().remove(f);
  |             t.addFolder(t);
  |             f.setParent(t);
  |         }
  |         session.persist(f); 
  | 		return f;
  | 	}
  | 

It seems like the last few lines should look like this instead:

  
  | ...
  |  if(folder.getId() != t.getId()) {
  |             f.getParent().getFolders().remove(f);
  |             t.addFolder(f);   // ---->> SUBTLE CHANGE HERE
  |             f.setParent(t);
  |         }
  | ...
  | 

Thoughts?  This is probably not an issue since the webmal doesn't appear to use this function yet, but should be fixed if its a bug.

View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=3987716#3987716

Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=3987716