Bugzilla – Full Text Bug Listing
| Summary: | Enhance the IServerWorkingCopy API to allow setting the timeout and publishers state | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP ServerTools | Reporter: | Troy Bishop <tjbishop> | ||||||||
| Component: | wst.server | Assignee: | Elson Yuen <eyuen7> | ||||||||
| Status: | CLOSED FIXED | QA Contact: | Angel Vera <arvera> | ||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P2 | Keywords: | plan | ||||||||
| Version: | 3.0.5 | Flags: | arvera:
review+
|
||||||||
| Target Milestone: | 3.2 M4 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| See Also: |
https://git.eclipse.org/r/108824 https://git.eclipse.org/r/108825 https://git.eclipse.org/r/109803 https://git.eclipse.org/c/servertools/webtools.servertools.git/commit/?id=e275a91a1c004e43564ac07236cbfc0add96ae53 |
||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Troy Bishop
Created attachment 139578 [details]
possible (but untested) patch to accomidate this request based on the 3_0_5_patches branch.
setting severity to 'enhancement' It would also be nice to also expose the 'setAutoPublishSetting' method currently on org.eclipse.wst.server.core.internal.ServerWorkingCopy to the IServerWorkingCopy interface as well. hmm.. so many API request make me think that perhaps we should have a way to allow general modifications of any properties, and perhaps prevent from updating those properties that are key for the framework to work. Created attachment 152513 [details]
v1.0
Adding a patch v1.0 that will provide general getters and setters for all attributes on the server. Angel, Please assign this bug to me if you find the latest patch is good. Created attachment 152545 [details]
v1.1
There is a problem with previous patch. Submitting a new one.
Comment on attachment 152545 [details]
v1.1
The patch looks good. Thank you
I accepted patch mostly as it is, but I added one more method that all the setAttribute are calling, to avoid the cases 'of shooting yourself in the foot'. The new method will prevent callers from modifying the id and the timestamp attribute of the server. I also noticed that this will break one of our JUnits, so I had to fix it by adopting the new methods. The breakage of the JUnit would indicate that adopters of IServer would be broken after this change, but the IServer interface clearly describe not to implement. So I am going to assume that noone is implementing it. Changes committed to HEAD, will release later For now I am allowing to modify the ID. In the existing API, the adoptors are allowed to change the ID of the server and runtime since they are inheriting the delegate and we allow the delegate to change the ID in the past. Adding the new constraint on blocking the API from changing the id will be a removal of function from the existig API. We have to be careful when adding new constraints to limit the usage of the API. The same API is being shared by the existing delegate code and the new API, therefore, whatever new constraint that we add to will pose a new limitation to the existing delegate code. In a sense, we may be starting to 'shoot ourselves on the foot' on the API level already by adding the new constraint on the implementation. I think the id field in a sense is more easy to understand. On the other hand, can you explain why changing the timestamp is categorized as a bad field to get modify? Although I can't come up with a usage scenario on modifying the timestamp, I don't see anything disaster will happen when you change the timestamp. Maybe there are reasons that I am not aware of that warrants an explicit block on changing the field. (In reply to comment #11) I see the ID field and the timestamp fields as internal. I can't think of any use case where one would want to modify those two fields, if you have one please let me know. I did remove the ID from being restricted because we are using the setAttribute method to initially set the ID and that brings problems. Ideally the creation of a new server would call some .internal code to set the ID of the server, and then restrict the ID from being modified, but that would be a future enhancement, that I need to see if I can fit in 3.2. Modifying the timestamp could potentially bring problems when ServerWorkingCopy.save() is called, as we have WorkingCopyHelper.validateTimestamp that checks that the timestamp of the ServerWorkingCopy is the same as the timestamp of its original. It is not a disastrous behaviour but it will throw a CoreException :) You are right that today, we allow the modification of this two fields. Introducing the new restriction qualify as new behaviour for the API which we are allow to do in 3.2. Perhaps a post on the newsgroup and dev mailing list is necessary once we agree that this is the right thing to do. Changes released to HEAD New Gerrit change created: https://git.eclipse.org/r/108824 New Gerrit change created: https://git.eclipse.org/r/108825 New Gerrit change created: https://git.eclipse.org/r/109803 Gerrit change https://git.eclipse.org/r/109803 was merged to [master]. Commit: http://git.eclipse.org/c/servertools/webtools.servertools.git/commit/?id=e275a91a1c004e43564ac07236cbfc0add96ae53 |