Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 332186 - ALLOW_CLASSPATH_DEP product preference should be deprecated
Summary: ALLOW_CLASSPATH_DEP product preference should be deprecated
Status: RESOLVED FIXED
Alias: None
Product: WTP Java EE Tools
Classification: WebTools
Component: jst.j2ee (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: 3.2.3   Edit
Assignee: Aidyl Kareh CLA
QA Contact: Chuck Bridgham CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 332684
  Show dependency tree
 
Reported: 2010-12-08 23:38 EST by Aidyl Kareh CLA
Modified: 2010-12-22 12:06 EST (History)
4 users (show)

See Also:
cbridgha: review+
jsholl: review? (ccc)


Attachments
Proposed Patch (23.83 KB, patch)
2010-12-08 23:41 EST, Aidyl Kareh CLA
no flags Details | Diff
Proposed Patch - Updated (25.03 KB, patch)
2010-12-09 13:51 EST, Aidyl Kareh CLA
no flags Details | Diff
patch v3 (12.42 KB, patch)
2010-12-15 15:17 EST, Jason Sholl CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aidyl Kareh CLA 2010-12-08 23:38:29 EST
Build Identifier: WTP 3.2.3

The "allowClasspathDep" preference was introduced by adopters to override the WTP behavior of doing dynamic manifest updates at runtime and export. However, the adopter that introduced this product preference no longer needs to override it so the related classes can be deprecated and the calls to the preference can be removed. After the calls to this preference have been removed, all existing functionality in WTP will continue working as it has. 

Adopters should still be able to disable the validator which checks for duplicate classpath and component URIs (call to validateDuplicateClasspathComponentURIs() method in the UIEarValidator class), so the product preference "validateDupClasspathCompURI" will be introduced for this purpose and its default value will be set to true.

Reproducible: Always
Comment 1 Aidyl Kareh CLA 2010-12-08 23:41:56 EST
Created attachment 184838 [details]
Proposed Patch
Comment 2 Chuck Bridgham CLA 2010-12-09 13:04:05 EST
I approve of this change - and it will be added to PMC for approval.  
It needs to be stressed that the original adopter of this code (IBM) no longer has a use, and recommend removing this option for simplicity.
Comment 3 Aidyl Kareh CLA 2010-12-09 13:51:30 EST
Created attachment 184887 [details]
Proposed Patch - Updated

Updated @deprecated comments.
Comment 4 Aidyl Kareh CLA 2010-12-09 15:04:16 EST
   * Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such.

The adopter that introduced the ALLOW_CLASSPATH_DEP product preference no longer needs to disable dynamic manifest updates at runtime and export and is recommending that the preference and related classes be deprecated and the calls to the preference removed for code clarity. We want to be able to ensure that all users will have consistent behavior regardless whether they are using existing workspaces or new ones or what the value of this preference might be. All existing functionality in WTP will remain unchanged by this patch since this preference was used to disable WTP functionality. 

    * Is there a work-around? If so, why do you believe the work-around is insufficient?  

No.

    * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 

Tested through UI.

    * Give a brief technical overview. Who has reviewed this fix?

The attached patch updates the code to deprecate the "allowClasspathDep" preference and the related classes/methods used for get/set purposes. This patch also removes the calls to check this preference's value. A new product preference, "validateDupClasspathCompURI", was also introduced by this patch to allow adopters to disable the validator which checks for duplicate classpath and component URIs. This patch has been reviewed by Chuck and Jason.

    * What is the risk associated with this fix?

Low since all WTP functionality will remain unchanged.
Comment 5 David Williams CLA 2010-12-09 16:15:13 EST
Can you net this out for me? Its not obvious why you are asking for PMC approval ... is it that the UI is changing? Or, simply since sort of a change to API, marking it deprecated?
Comment 6 Aidyl Kareh CLA 2010-12-10 02:15:08 EST
Since we are deprecating and removing the use of the preference, other adopters that might have used this preference expecting that it disabled the 'dynamic manifest updates' functionality will now see a change in behavior. There is no change in UI, WTP functionality will remain unchanged, and adopters code will not break by these changes.
Comment 7 David Williams CLA 2010-12-10 03:47:10 EST
(In reply to comment #6)
> Since we are deprecating and removing the use of the preference, other adopters
> that might have used this preference expecting that it disabled the 'dynamic
> manifest updates' functionality will now see a change in behavior. There is no
> change in UI, WTP functionality will remain unchanged, and adopters code will
> not break by these changes.

I am not sure why to remove it in maintenance stream, if there is any chance it would break someone. I could see how you might want to mark it deprecated, and document it to be removed in 3.3 stream ... but, just wondering. 

I'd guess there's little chance of anyone else using it? But ... why take a chance, especially in maintenance? 

Your statements above seems a little contradictory ... staring by saying there might be a change in behavior if adopters used it ... but then saying adopters code would not be broken? 

(Sorry if I'm being dense ... maybe it'd be obvious if I loaded up the code, applied the patch, etc, ... but, since you asked for PMC review ... you are going to get it :)
Comment 8 Jason Sholl CLA 2010-12-15 15:13:59 EST
Retracting PMC approval; we are taking a route which will not risk regression in 32M for 3.2.3.  The plan is still to remove this code in HEAD for 3.3.
Comment 9 Jason Sholl CLA 2010-12-15 15:17:29 EST
Created attachment 185261 [details]
patch v3

Updated patch deprecates but does not change any behavior.  Also introduces a new product constant to control whether the EAR Validator attempts to load classpath references.
Comment 10 Jason Sholl CLA 2010-12-15 15:20:47 EST
created bug 332684 to delete this preference from HEAD
Comment 11 Jason Sholl CLA 2010-12-15 15:25:46 EST
Code checked into both 32M and HEAD for WTP 3.2.3 and 3.3
Comment 12 Chuck Bridgham CLA 2010-12-22 12:06:19 EST
approved