Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 316369

Summary: ServiceProviderConfigurationWidget unable to save changes
Product: [Tools] PTP Reporter: Vivian Kong <vivkong>
Component: Service ModelAssignee: Vivian Kong <vivkong>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: g.watson
Version: 4.0Flags: g.watson: review+
Target Milestone: 4.0.1   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
patch for ptp_4_0
none
revised patch for ptp_4_0
none
patch for HEAD
none
additional changes needed for HEAD none

Description Vivian Kong CLA 2010-06-09 15:51:36 EDT
Changes to service provider were not save after they were made in the ServiceProviderConfigurationWidget (used in service configurations property/preference pages).

Upon selecting a service from the service tree, an IServiceProviderWorkingCopy was created from an IServiceProvider and an IServiceProviderContributor was setup to configure the working copy.  However the IServiceProviderContributor was actually editing the IServiceProvider and not its working copy.  Therefore, when user clicks OK, the unmodified working copy would attempt to save its properties to the provider, which set the provider back to its original state.
Comment 1 Vivian Kong CLA 2010-06-21 12:51:17 EDT
Created attachment 172343 [details]
patch for ptp_4_0
Comment 2 Vivian Kong CLA 2010-06-21 12:54:24 EDT
Hi Greg, can you please review this patch?  Without this patch, remote services cannot be edited for remote projects.

Should the IServiceProviderContributor interface be changed so that configureServiceProvider() works with IServiceProviderWorkingCopy instead of an IServiceProvider?
Comment 3 Vivian Kong CLA 2010-06-21 12:55:17 EDT
Created attachment 172344 [details]
revised patch for ptp_4_0
Comment 4 Greg Watson CLA 2010-06-22 10:37:16 EDT
This is working for me, so I'm ok with the patch.

It would seem to make sense if IServiceProviderWorkingCopy was passed to configureServiceProvider(). I guess this can only be changed in head.
Comment 5 Vivian Kong CLA 2010-06-22 13:49:54 EDT
(In reply to comment #4)
> This is working for me, so I'm ok with the patch.
> 
> It would seem to make sense if IServiceProviderWorkingCopy was passed to
> configureServiceProvider(). I guess this can only be changed in head.

Thanks for reviewing the patch Greg.

I'll post another patch for HEAD with the API change.
Comment 6 Vivian Kong CLA 2010-06-22 14:19:41 EDT
Created attachment 172446 [details]
patch for HEAD
Comment 7 Greg Watson CLA 2010-06-23 16:09:58 EDT
Looks good. Please apply.
Comment 8 Vivian Kong CLA 2010-06-24 10:28:47 EDT
Fixed in ptp_4_0 and HEAD.

I've also updated the plugin version.
Comment 9 Vivian Kong CLA 2010-06-30 09:14:51 EDT
Created attachment 173093 [details]
additional changes needed for HEAD
Comment 10 Vivian Kong CLA 2010-06-30 09:15:33 EDT
Additional changes applied to HEAD
Comment 11 Greg Watson CLA 2010-06-30 11:26:05 EDT
I've also fixed the RM's that are using this interface.