Bug 199871

Summary:

LocalFileService needs to implement getMessage()

Product:

[Tools] Target Management

Reporter:

Kevin Doyle <kjdoyle>

Component:

RSE

Assignee:

Kevin Doyle <kjdoyle>

Status:

CLOSED FIXED

QA Contact:

Martin Oberhuber <mober.at+eclipse>

Severity:

normal

Priority:

CC:

dmcknigh

Version:

2.0

Keywords:

contributed

Target Milestone:

2.0.1

Hardware:

OS:

Windows XP

Whiteboard:

Bug Depends on:

Bug Blocks:

210555

Attachments:

Description	Flags
Implemented getMessage for LocalFileService and LocalProcessService	mober.at+eclipse: iplog+

Description Kevin Doyle

2007-08-14 09:19:58 EDT

LocalFileService does not implement getMessage() and AbstractFileService.getMessage() just returns null.

This is causing NPE's when operations fail.

-----------Enter bugs above this line-----------
TM 2.0.1 Testing
installation : eclipse-SDK-3.3
RSE install  : Dev Driver - 
java.runtime : Sun 1.5.0_11-b03
os.name:     : Windows XP, Service Pack 2
------------------------------------------------

Comment 1 Martin Oberhuber

2007-08-14 09:42:44 EDT

LocalFileService is the only user of #getMessage(String).
LocalProcessService#getMessage(String) has the same problem and must be fixed.

The method is being declared in IService, but not documented. So it's unclear what it's being used for and what the interface requires it to return. We should file a separate bug that API Docs are needed for IService - Kevin can you please do that.

It looks like in the end it should work like AbstractDStoreService#getMessage(String). Only problem is, that for dstore the ISystemMessageProvider is being set, whereas for the other services this is not done.

Fortunately, LocalFileService and LocalProcessService are both "internal", so what we need to do is:
  * Create an additional constructor for both, which takes the
    ISystemMessageProvider as additional argument
  * Pass the RSEUIPlugin.getDefault() as the ISystemMessageProvider argument
    at the place where the service is constructed
  * Override getMessage() with an implementation similar to 
    AbstractDStoreService. In order to be backward compatible, we should 
    return super.getMessage() in case the messageProvider has not been set 
    (==null).

Kevin can you do that?

Comment 2 Kevin Doyle

2007-08-14 09:59:30 EDT

I'll do it.  Thanks for writeup on it.

Comment 3 Kevin Doyle

2007-08-16 11:55:49 EDT

Created attachment 76232 [details]
Implemented getMessage for LocalFileService and LocalProcessService

Martin, can you review and commit the patch?

I also opened bug #200206 for adding api doc's to IService.

Legal Message: I, Kevin Doyle, declare that I developed attached code from
scratch, without referencing any 3rd party materials except material licensed
under the EPL. I am authorized by my employer, IBM Canada Ltd. to make this
contribution under the EPL.

Comment 4 Martin Oberhuber

2007-08-17 07:38:36 EDT

Patch applied, thanks!

Note that in Meta-info/Manifest.mf for subsystems.files and subsystems.processes,
these now have to require version 2.0.1 of services.files at least, since the "internal" API changed to provide the new constructor.

Comment 5 Kevin Doyle

2007-09-13 11:14:22 EDT

Verified fixed with 2.0.1RC1.