Community
Participate
Working Groups
Build ID: I20090313-0100 CDO 2.0.0M6 The current CDOObject.isLocked() means the current CDO view owns or not a lock on this object. It's worth having a new method to be able to know if an object is locked by other views. See post : http://www.eclipse.org/newsportal/article.php?id=40578&group=eclipse.tools.emf#40578. Eike suggests to keep the current isLocked() method as is to avoid breaking existing code and to add a new method isLockedByOthers() for this new semantic.
I've just finished to implement the isLockedByOthers feature. Would you like to have the code ? I do not have access to the CVS repository from my Eclipse SDK at office :( Nevertheless, I can provide you with a zip file containing all cdo projects I modified. You will have to search a specific TODO task identified by this bug url (i.e https://bugs.eclipse.org/bugs/show_bug.cgi?id=270415)
(In reply to comment #1) > I've just finished to implement the isLockedByOthers feature. > > Would you like to have the code ? > I do not have access to the CVS repository from my Eclipse SDK at office :( > Nevertheless, I can provide you with a zip file containing all cdo projects I > modified. You will have to search a specific TODO task identified by this bug > url (i.e https://bugs.eclipse.org/bugs/show_bug.cgi?id=270415) > Yes please attached it to this bugs! I will be very happy to review it! Simon
Again, I am sorry to be not able to provide you with a patch. You can find in this bugzilla https://bugs.eclipse.org/bugs/show_bug.cgi?id=265136 a zip file that contains different CDO projects with different fixes I made. To get the fixe for this bugzilla search for a Task named : FIX https://bugs.eclipse.org/bugs/show_bug.cgi?id=270415
Hi Simon, Did you retrieve the code that implements isLockedByOthers() ? Stéphane.
Yes I looked at it briefly.. but I didn't extract it so far from the project you submitted. I will do it as soon as I have time ! 1 - Extract the patch from project that Stephane provide 2 - Create testcases 3 - Change java doc! 4 - Review by Eike! I will start step 1 as soon as I can. Simon
To help you, I will make a real patch by the end of the week-end. I noticed, I forgot to give you the new Indication class I added in org.eclipse.emf.cdo.server :( Stephane.
This will be awesome!
Created attachment 130887 [details] Fix for this enhancement Hi Eike and Simon. I've just finished a real patch :) I do hope you are happy guys. I'm not a patch expert, hence I do hope the file contains all the materials. Let me know if you can manage it. This patch should contain code modifications and 2 new classes : one for the request and the other one for the Indication. You will find after integration 19 TODO task containing this bugzilla number. Stephane
Created attachment 130908 [details] Patch v2 I consolidated the network protocol and a reformatted everything. I added test cases in LockingManagerTest, which are failing. The distiction between read locks and write locks gets lost somewhere. Stephane, can you dig a bit deeper? Thx for your effort ;-)
Simon, can you please also have a look at this patch?
Created attachment 130912 [details] Fix issue between ReadLock and WriteLock Hi Eike, You are right, there is a mess, between read locks and write locks. When I made the first patch, I missed one thing. writeLocks and readLocks are held in the same collection. Hence, RWLockManager#getLockEntry method gets existing locks for given object. Even if the right lock strategy is retrieved we can't unify the call to the LockEntry. In fact, we need two methods (that's already the case for isLocked()) LockEntry<K, V> entry = getLockEntry(objectToLock); This new patch (changes in RWLockManager.java) in addition to your one, fixes the bug. Let me know the result of your test case. Stephane.
I will review this two patchs before the end of the day. I look at them quickly and I think something is missing.
Created attachment 130916 [details] Patch v3 RWLockManager had 2 issues. - ReadLockEntry.isReadLockByOthers - WriteLockEntry.isReadLockByOthers I change one testcase as well to test that.
Hi Simon, I don't understand why in your patch, I didn't see the changes I did today to solve the issue pointed by Eike ? Did you have a try to the last patch I submitted ? Did you fix the issue pointed by Eike in another way ? Stephane.
Simon, I introduced two methods as you did to implement isLocked(). The problem is that getLockEntry(object) (called from hasLockByOther()) returns a LockEntry without testing against the requested lock type. That's why you need 2 methods afterwards. The ReadLockStrategy tests against isReadLock() while the WriteLockStrategy tests again isWriteLock(). I tried to implement isLockByOthers in the same way in the last patch I submitted. Stephane.
Guys, My mistake ! Forget my last two message, I made an error in applying the patch v3. Hence, I did get the code submitted by Simon. Sorry to disturb you with useless messages ;) Stephane.
Let us know if this path doesn't work well for you. Simon
It still works well for me. Your test cases are very useful to have a good code coverage :) When will you commit the code to HEAD ?
As soon as Eike review the final patch!!!
Created attachment 130930 [details] Patch v3 Ready to be committed
Wait a minute! This patch contains a substantial contributionby non-committers and we need to follow the IP process: Confirm on Bugzilla that the Contributor: (a) wrote 100% of the code; (b) that they have the right to contribute the code to Eclipse; and (c) the file header contains the EPL License header. Stephane, can you provide your company name and confirm the above items? I filed CQ: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=3234
I will wait until the CQ is completed. I thought we only need to do that if more than 250 lines ? Simon
Hi, To answer your questions : I'm working for Thales. a) I wrote 100% of the code contained in the patch I submitted. For b) and c) I will check that tomorrow morning with my boss. To be honest, I don't think there is a problem : 1) My company is submitting a proposal (http://www.eclipse.org/proposals/egf/) to start a new EMFT component project and is ready to give the initial code contribution. This code contribution is far more important than the lines I give you :). In the proposal, you can find my business e-mail ;). Guillaume Brocard (working on Oracle CDO db adapter) and I, are the guys who wrote the EGF code. For 2 months, we've moved on a new project that needs CDO. 2) We need the isLockedByOthers feature that's why I give it a try. It's really better if the feature is included in CDO rather than some proprietary code where you found bugs. I let you know asap for b) and c) questions. Stéphane.
c) is already clear. IIRC after our rework, there are no new files at all.
Hi guys, I've just checked with my Hierarchy. No problem to contribute the code to Eclipse. Stéphane.
Great! In the meantime I 've withdrawn the CQ since only 29 LOC of your initial contribution made it to the final patch (250+ requires a CQ). That makes it even easier for us. Thank you again for your engagement! Committed to HEAD.
Hi guys, I'm back to business :) The isObjectLockedByOthers method works fine. Nevertheless, I'm wondering if we could improve it in returning a list of view ids rather than a boolean. If the returned list is empty that means no lock, otherwise we have the lockers. From my opinion, It could be useful to know, what are the views that lock the related object. Let me know what you think about that and I can have a try to implement it. Stephane
(In reply to comment #27) > Hi guys, > Hi Stephane, > I'm back to business :) Good!!! > The isObjectLockedByOthers method works fine. Perfect. > Nevertheless, I'm wondering if we could improve it in returning a list of view > ids rather than a boolean. If the returned list is empty that means no lock, > otherwise we have the lockers. > > From my opinion, It could be useful to know, what are the views that lock the > related object. Could remote view could be useful ? I understand your point and I think it could be valid. I don`t see the business case at the moment. :-) You can create a bugzilla :-). > > Let me know what you think about that and I can have a try to implement it. > > Stephane >
Hi Simon, I've filed a bugzilla to discuss the comment #28 : https://bugs.eclipse.org/bugs/show_bug.cgi?id=273411. Stéphane.
Fix available in EMF CDO 2.0.0M7
Generally available.
