How about this? I didn't know where to add the Mutex, and I didn't think ResourceData would be appropriate. So, I implemented the Resource::Private class and added it over there. This should not break binary compatibility as it already existed. Or is there a better way?
- Vishesh Handa On Sat, May 29, 2010 at 2:25 PM, Vishesh Handa <[email protected]> wrote: > Hey Sebastian > > I'm sorry for the late reply I was busy with college stuff. > > I looked at the modified patch, and I like the fact that it works, but I > don't like the fact that we have to remember additional stuff. > DetermineUri() must not me called inside ResourceData, and that you'll > probably have to check if the ResourceData is stored() and its uri has been > determined, before calling a method. These small things then to add up, and > clutter the Resource Code. > > Ideally I would have liked ResourceData to do all this checking itself, so > that when altering the Resource class we don't have to worry about it. ( But > then we already had to worry about store() ) I don't have a solution, but > I'm trying to figure out one. > > I never even thought about multi-threading before this, and, yes, I too > think it will crash. ( Time to add more tests? Can we add multi-threading > tests? ) I was thinking of some way of checking if the determineUri lock > exists, and then accordingly calling determineUri() from Resource, but I > think your method of mutex locking the Resource should cover it. > > I'll try to implement it (It pretty simple, right?) > > - Vishesh Handa > > > > > On Thu, May 27, 2010 at 5:29 PM, Sebastian Trüg <[email protected]> wrote: > >> Hi Vishesh, >> >> I finally looked at the remove-proxy patch again and fixed it. Now the >> only problem left is the design weirdness of ResourceData instances >> deleting themselves. >> >> The main thing I did was removing all calls to determineUri from >> ResourceData and moving them into Resource. I did the same with load() >> and store() which is actually not necessary. Maybe it would be cleaner >> to move those back... >> >> I did not look into the merging of load and determineUri yet. I think we >> should do that in a separate patch only after we cleaned this one up. >> Otherwise it is impossible to debug. >> >> Cheers, >> Sebastian >> >> On 05/26/2010 12:52 PM, Vishesh Handa wrote: >> > >> > >> > On Wed, May 26, 2010 at 3:13 PM, Sebastian Trüg <[email protected] >> > <mailto:[email protected]>> wrote: >> > >> > On 05/26/2010 10:08 AM, Vishesh Handa wrote: >> > > > 3. When checking if the the m_kickoffUri is a nie:url for a >> > resource. >> > > > The nieUrl is assigned to be equal to the uri. This however >> > gets fixed >> > > > in the load(). And nieUrl isn't used anywhere, so it doesn't >> > > matter that >> > > > much. >> > > >> > > I suppose you mean this part: >> > > >> > > if( it.next() ) { >> > > QUrl uri = it["r"].uri(); >> > > if( uri.isEmpty() ) { >> > > m_uri = m_kickoffUri; >> > > } >> > > else { >> > > m_uri = uri; >> > > m_nieUrl = uri; >> > > } >> > > >> > > The last two lines. AFAICT this is perfectly fine since in the >> > latter >> > > case both nie:url and resource URI are equal. >> > > >> > > >> > > I respectfully disagree. :) >> > > >> > > The query used is this "select distinct ?r ?o where { { ?r nie:url >> > <uri> >> > > . } UNION { <uri> ?p ?o . } } LIMIT 1". The case where ?r isn't >> empty >> > > is when the <uri> contains the nie:url and therefore ?r will >> > contain the >> > > resource uri. >> > >> > You are of course correct. It should be "m_nieUrl = m_kickoffUri" >> > instead. Agreed? >> > >> > >> > Yup. :) >> > >> > If you get the time could you please look at my horrible merge patch? >> > >> > Thanks >> > - Vishesh Handa >> > >> > Cheers, >> > Sebastian >> > >> > >> > >
removeProxyPatch6
Description: Binary data
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
