[Nepomuk] Nepomuk Core - Questions & Patches

[Vishesh Handa]

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 <handa.vish at gmail.com> 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 <trueg at kde.org> 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 <trueg at kde.org
>> > <mailto:trueg at kde.org>> 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
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20100529/8ff8e333/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: removeProxyPatch6
Type: application/octet-stream
Size: 24424 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/nepomuk/attachments/20100529/8ff8e333/attachment-0001.dll