Skip to content

Conversation

@simonLouvet
Copy link
Contributor

fix #1011

@simonLouvet simonLouvet requested a review from srosset81 July 8, 2022 14:13
Copy link
Contributor

@srosset81 srosset81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci pour le fix ! Juste deux petits commentaires.

let type = triples.find(q => ['http://www.w3.org/1999/02/22-rdf-syntax-ns#type'].includes(q.predicate.value))
.object.value;

if (['http://semapps.org/ns/core#File', 'semapps:File'].includes(type) && !forceSemantic) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comme bodyToTriples retourne des quads, ce ne sera jamais semapps:File mais toujours des URIs complets. Même chose pour mimeType et localPath.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Par ailleurs, ce serait bien de mettre la condition if (!forceSemantic) avant l'appel à bodyToTriples, ça évite de faire ces calculs coûteux (forceSemantic est souvent true, par exemple lorsque ldp.resource.get est appelé par ldp.container.get)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j'ai eu le cas d'un semapps:Filec'est pour ca que j'ai pris des précautions. Je crois que c'est dans le cas d'un json-ld avec prefix/context

@simonLouvet simonLouvet requested a review from srosset81 July 11, 2022 10:39
@@ -0,0 +1,4 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si tu peux effacer ce fichier inutile STP

Base automatically changed from next to master November 23, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants