Removed unchecked chained methods.

sspeiche commented 10 years ago

I must confess to not know what problem this is solving. I see you are ensuring that the values are URIs but just ignoring if there exists a triple with ldp:insertedContentRelation but value is not a URI. Seems like this should be an error, not continue. Can you explain a little more what the problem you are hitting is and how this overcomes it?

MiguelAraCo commented 10 years ago

I don't think that if the resource is not found the method should fail. And because it wasn't doing any checking of some kind, it was failing in a hard to debug way.

To be honest I don't see a reason for this method to exist. It seems its purpose is to return the "primaryTopic" of a resource instead of the actual resource. It appears that this could be useful when working with indirectContainers, but there are a few things that don't quite match:

foaf:primaryTopic is just one of the possible objects in an ldp:insertedContentRelation triple, and this method is assuming if you are not using ldp:MemberSubject, then you are using foaf:primaryTopic.
It's trying to return a Resource that is found as an object, in a triple that has for a subject the containerURI and for the predicate the object from the ldp:insertedContentRelation triple, which to me doesn't make sense. First of all, we can't assume the resource is actually present in the model that we are handling. Secondly, that triple shouldn't actually exist. IndirectContainers create a membership triple, using the object of the ldp:insertedContentRelation triple, that has for a subject the membershipResourceURI, as the predicate the object of the ldp:hasMemberRelation triple, and for an object a resource that depends on the ldp:insertedContentRelation object.

Sorry if this is confusing, I tried my best in explaining it clear enough.

sspeiche commented 10 years ago

I've had a chance to review this method in more detail and discuss with some. It would be good for you to post a couple of the RDF that is causing the method to error.

Let me address some of your points:

the purpose of the method is to find the "primary" resource URI to do some tests against, such as using it as the subject for triples to insert for PUT testing. The name "primary topic" might be confusing but is roughly the same meaning.
foaf:primaryTopic is not hard coded, it is just the name of the temporary variable holding the result
it looks like there could be an error when there is incorrectly an triple whose predicate is ldp:insertedContentRelation and object is not a URI resource. We can fix the method code here but also looks like the server might be generating the wrong triple or was created in error.

After evaluating this in more detail, I don't think this whole ldp:insertedContentRelation code is needed. Since there member resource URI and container URI are not both given to the method, these triples should rarely occur in a resource in such a way that it could determine the primary resource. After I see your RDF sample and do a little more evaluation, I would probably just remove that logic altogether.

MiguelAraCo commented 10 years ago

Now that you closed it, should I open a new one to share the RDF you asked for?

sspeiche commented 10 years ago

Add it here.

MiguelAraCo commented 10 years ago

+1 for removing all of this logic. This is the model the method is currently handling when it throws an exception:

@base <http://carbonldp.com/apps/ldp-testsuite/membershipResource/indirectContainer/>.
@prefix ldp: <http://www.w3.org/ns/ldp#>.
@prefix c: <http://carbonldp.com/ns/v1/platform#>.
@prefix xsd: <http://www.w3.org/2001/XMLSchema#>.
@prefix foaf: <http://xmlns.com/foaf/0.1/>.
<>
    a   ldp:IndirectContainer;
    c:created   "2014-10-29T12:37:58.130-05:00"^^xsd:string;
    c:defaultRetrievePreference 
        ldp:PreferContainment,
        ldp:PreferMinimalContainer;
    c:modified  "2014-11-04T11:40:53.262-06:00"^^xsd:dateTime;
    http://example.com/ns#comment   "modified";
    ldp:contains    
        <1443994488360126150>,
        <2378047310517808607>,
        <3045364954094698882>,
        <4083305103342022698>,
        <4670849290323396662>,
        <5721007932776501478>,
        <6302739556100581025>,
        <7037787392880241375>,
        <8632633096817430694>,
        <9161483135061249633>;
    ldp:hasMemberRelation   http://example.org/ns#indirectMember;
    ldp:insertedContentRelation foaf:primaryTopic;
    ldp:membershipResource  http://carbonldp.com/apps/ldp-testsuite/membershipResource.

w3c / ldp-testsuite

Removed unchecked chained methods. #213