Hey Sebastian<br><br><div class="gmail_quote">On Tue, May 25, 2010 at 11:18 PM, Sebastian Trüg <span dir="ltr"><<a href="mailto:trueg@kde.org">trueg@kde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Hi Vishesh,<br>
<br>
no, I do not want to keep determineUri. That is the whole point. And why<br>
is it buggy?<br>
<br></blockquote><div><br>Minor things. <br><br>1. If I call Resource( QString("file:/whatever") ) and whatever is actually a filex:/ url. It won't get identified, cause the filex:/ searching is only applicable to QUrls. <br>
<br>2. Code like this is perfectly legal, and works (no modifications required)<br><br><div style="margin-left: 40px;">Resource r1( KUrl("nepomuk:/res/blah-blah") );<br>r1.setRating( 5 );<br>qDebug() << r1.rating();<br>
</div><br>This is because determineUri() accepts uris whose scheme is "nepomuk", but don't really exist in the database. Should this be allowed?<br><br>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.<br>
<br>-----------<br><br>I'll start combing them ( The code becomes ugly! :-( )<br><br>- Vishesh Handa<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Cheers,<br>
<font color="#888888">Sebastian<br>
</font><div class="im"><br>
On 05/25/2010 07:36 PM, Vishesh Handa wrote:<br>
><br>
> Hey Sebastian<br>
><br>
> On Mon, May 24, 2010 at 4:43 PM, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a><br>
</div><div><div></div><div class="h5">> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>> wrote:<br>
><br>
><br>
> Yeah... I think merging determineUri into load and calling it simply<br>
> load() is perfectly fine. The that method would only be called from<br>
> Resource but never in ResourceData methods.<br>
><br>
><br>
> Just to be clear you want to merge determineUri() and load() and call it<br>
> load()?<br>
><br>
> Do you still want to keep the original determineUri? Cause merging it<br>
> would cause massive code duplication, if we still plan on keeping<br>
> determineUri() (which is a little buggy, btw) And if we don't plan on<br>
> keeping determineUri() what do we do with all the calls to it? It called<br>
> before all of the property, hasProperty() functions, and is called in<br>
> store(), which is called whenever you have any kind of setters(<br>
> setProperty() setType() .. )<br>
><br>
> After trying to merge them. I don't think merging is that simple.<br>
><br>
> - Vishesh Handa<br>
><br>
><br>
><br>
> Cheers,<br>
> Sebastian<br>
><br>
> > - Vishesh Handa<br>
> ><br>
> ><br>
> ><br>
> > Be aware though that I did not look at your patch in detail<br>
> yet. I will<br>
> > do that later. So I might have missed something. :)<br>
> ><br>
> > Cheers,<br>
> > Sebastian<br>
> ><br>
> > On 05/23/2010 05:54 PM, Vishesh Handa wrote:<br>
> > ><br>
> > > On Sun, May 23, 2010 at 8:20 PM, Sebastian Trüg<br>
> <<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> > <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>><br>
> > > <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>>>> wrote:<br>
> > ><br>
> > > On 05/23/2010 04:23 PM, Vishesh Handa wrote:<br>
> > > > > One way of solving both the problems is to derive<br>
> > > ResourceData from<br>
> > > > > QObject. And then call deleteLater() instead of<br>
> "delete<br>
> > > this". (Patch<br>
> > > > > attached)<br>
> > > ><br>
> > > > oh, no, please don't. Totally useless overhead. It<br>
> is much<br>
> > > simpler to<br>
> > > > make the mutex recursive.<br>
> > > ><br>
> > > ><br>
> > > > Well then we need a better way of solving the "delete<br>
> this"<br>
> > problem.<br>
> > > > When you say moving "it" into Resource, do you mean<br>
> > determineUri() or<br>
> > > > replaceWith() ?<br>
> > ><br>
> > > I mean all calls of determineUri().<br>
> > > Example:<br>
> > > ResourceData::removeProperty calls determineUri. Instead<br>
> > determineUri<br>
> > > should be called in Resource::removeProperty.<br>
> > ><br>
> > ><br>
> > > I'm sorry. I don't understand. Isn't it the same thing? If A<br>
> calls<br>
> > B or<br>
> > > B is called inside A?<br>
> > ><br>
> > ><br>
> > > The only problem here is load. But I recently thought<br>
> that it<br>
> > could be<br>
> > > merged into determineUri (or the other way around).<br>
> After all most<br>
> > > resources do not have more than - say - 20 properties.<br>
> That is<br>
> > quickly<br>
> > > loaded and would avoid performing yet another query. What I<br>
> > mean is that<br>
> > > in determineUri<br>
> > ><br>
> > > select distinct ?r ?o where {<br>
> > > { ?r nie:url <uri> . }<br>
> > > UNION { <uri> ?p ?o . } } LIMIT 1<br>
> > ><br>
> > > could become something like<br>
> > ><br>
> > > select distinct ?r ?pp ?oo where {<br>
> > > { ?r nie:url <uri> .<br>
> > > ?r ?pp ?oo . FILTER(?r!=<uri> . }<br>
> > > UNION { <uri> ?p ?o . <uri> ?pp ?oo . }<br>
> > ><br>
> > > to determine the URI AND load all properties at the same<br>
> time.<br>
> > > Just an idea I had when I thought about ways to spped it<br>
> all up...<br>
> > ><br>
> > ><br>
> > > Seems logical. But then you call determineUri everywhere? I<br>
> guess we<br>
> > > could make it determineAndLoad(). :-)<br>
> > ><br>
> > > ---------------------------<br>
> > ><br>
> > > I had another idea on how to get rid of the proxy mess. I<br>
> was going to<br>
> > > tell you about it later, as its aim was to get rid of the<br>
> nasty lists<br>
> > > and determineUri mess, but it seems as though it would solve<br>
> the proxy<br>
> > > problem as well. (Nice side effect!)<br>
> > ><br>
> > > The ResourceManagerPrivate has 2 sets of lists. I'm going to<br>
> call them<br>
> > > INIT-LIST (m_initializedData) and the HALF-INIT list<br>
> > (m_uriKickoffData &<br>
> > > m_idKickoffData). Whenever a Resource is created by the<br>
> RMP::data()<br>
> > > function, it checks if it is present INIT-LIST or HALF-init<br>
> list, and<br>
> > > accordingly gives us a RD (ResourceData).<br>
> > ><br>
> > > A RD can go into the HALF-INIT lists in 4 ways - (look at<br>
> > determineUri)<br>
> > > --- KickOffId<br>
> > > ------- nepomuk://res/ stored as a string<br>
> > > ------- nao:identifier<br>
> > ><br>
> > > --- KickOffUri<br>
> > > -------- nie:url<br>
> > > -------- nepomuk://res/<br>
> > ><br>
> > > *Proposal :*<br>
> > > Remove the KickOffId completely. And make KickOffUri contain the<br>
> > > nao:identifier as well. When determineUri is being called, say<br>
> > > m_kickOffUri is the nepomuk resource uri, it should add<br>
> itself to the<br>
> > > INIT-LIST and it should add all other ways of getting<br>
> Initialized<br>
> > to the<br>
> > > HALF-INIT list ( nao:identifier and nie:url ) This way<br>
> RMP:data() will<br>
> > > always returns the correct RD* and a proxy wouldn't be required.<br>
> > ><br>
> > > determineUri would be a lot simpler -<br>
> > ><br>
> > > if( m_kickOffUri.isValid() ) {<br>
> > > // check for nepomuk://res/<br>
> > > // check for nie:url and the filex crap which I don't<br>
> understand<br>
> > > } else {<br>
> > > // check for nao:identifier<br>
> > > }<br>
> > ><br>
> > > An additional thing we could do is to make removeProperty()<br>
> > removing the<br>
> > > corresponding RD from the HALF-INIT list if the property<br>
> being removed<br>
> > > is nao:identifier or nie::url. (addProperty should check for the<br>
> > same)/<br>
> > ><br>
> > > I hope I've been clear enough.<br>
> > ><br>
> > > - Vishesh Handa<br>
> > ><br>
> > ><br>
> > > Cheers,<br>
> > > Sebastian<br>
> > ><br>
> > ><br>
> ><br>
> ><br>
><br>
><br>
</div></div></blockquote></div><br>