Bug 219975 - [api][breaking] Get rid of invalid implementation of SystemMessage#clone()
Summary: [api][breaking] Get rid of invalid implementation of SystemMessage#clone()
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.0 M6   Edit
Assignee: Martin Oberhuber CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2008-02-22 10:57 EST by Martin Oberhuber CLA
Modified: 2008-02-22 16:13 EST (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2008-02-22 10:57:08 EST 
The SystemMessage#clone() method is incorrectly implemented:
  * class SystemMessage does not implement the Cloneable interface
  * SystemMessage#clone() is only protected so only subclasses could call it
  * It's documentation does not state that substitutions are not cloned
  * Result of the clone is always a SystemMessage and not the runtime type
    of an extending class

In the end, cloning a SystemMessage is risky because the Substitutions Object[] array may contain objects of complex type that cannot be cloned (and would thus throw a CloneNotSupportedException at runtime). 

Therefore, it is better to get rid of the method - it's unclear what the use of cloning a SystemMessage would be, anyways.

This is a breaking API change because subclasses of SystemMessage could call clone(). But this is unlikely since SystemMessage was never meant to be extended.
Comment 1 Martin Oberhuber CLA 2008-02-22 11:00:08 EST 
As a first step, I'm committing a fixed version of SystemMessage#clone() for educational purpose:

[219975] Fix SystemMessage#clone()
   SystemMessage.java   v1.11
   SimpleSystemMessage.java   v1.4
   SystemUIMessage.java    v1.3
Comment 2 Martin Oberhuber CLA 2008-02-22 11:02:51 EST 
In the second step, I'm getting rid of SystemMessage#clone():

[219975][api][breaking] Get rid of SystemMessage#clone()
   SystemMessage.java   v1.12
   SimpleSystemMessage.java   v1.5
   SystemUIMessage.java    v1.4
Comment 3 Martin Oberhuber CLA 2008-02-22 16:13:35 EST 
Found some other places where clone() was not properly implemented or 
documented. There are also API changes involved:

* RemoteFile - removed "Cloneable" from implements
* SystemRemoteResourceInfo - removed "Cloneable" from implements

These are not really an API changes, since Cloneable is just a markup interface so only runtime behavior is changed; and, those classes could never properly have been cloned because their state includes complex objects which have never been properly cloned before. So, if CloneNotSupportedException occurs at runtime, it will only be an indication that incorrect code has been exposed.

[219975] Fix implementations of clone()
  SystemRemoteResourceInfo
  TarEntry
  HostProcessFilterImpl
  SystemFileTransferModeMapping
  RemoteFileFilterString
  RemoteFile
  RemoteCommandFilterString
  SystemCompileCommand