feat(api-v2): Erase a resource #1429

benjamingeer · 2019-09-04T14:41:14Z

This PR introduces a route /v2/resources/erase, for erasing a resource from the triplestore.

Resolves #1421.

tobiasschweizer · 2019-09-09T16:20:59Z

Have you included https://en.wikipedia.org/wiki/Eraser_(film) in the docs? :-)

benjamingeer · 2019-09-10T06:50:04Z

@tobiasschweizer Sorry, I only include references to films I've seen. :)

tobiasschweizer · 2019-09-10T08:37:52Z

What happens if the resource (e..g, a StillImageRepresentation) has an attached file that is stored in Sipi?

benjamingeer · 2019-09-10T08:40:25Z

What happens if the resource (e..g, a StillImageRepresentation) has an attached file that is stored in Sipi?

Currently Knora never deletes files from Sipi, and I think this is still the safest policy, because of use cases like #1078.

benjamingeer · 2019-09-10T08:41:21Z

(from Sipi's permanent storage I mean)

tobiasschweizer · 2019-09-11T08:05:32Z

webapi/src/test/scala/org/knora/webapi/e2e/v2/ResourcesRouteV2E2ESpec.scala

@@ -956,6 +956,38 @@ class ResourcesRouteV2E2ESpec extends E2ESpec(ResourcesRouteV2E2ESpec.config) {
                )
           }
        }
+
+        "erase a resource" in {


Could you add a test attempting to erase a resource that is referred to to make sure the request is rejected?

There's a test for that in ResourcesResponderV2Spec:

https://github.com/dhlab-basel/Knora/pull/1429/files#diff-b1ef5075da04ef708a48de2a6b007436R2062

Great. Sorry, I missed this one.

tobiasschweizer · 2019-09-11T08:12:45Z

webapi/src/main/twirl/queries/sparql/v2/isEntityUsed.scala.txt

 * @param ignoreKnoraConstraints if true, knora-base:subjectClassConstraint and knora-base:objectClassConstraint will be ignored.
 * This is necessary when modifying the cardinalities of a class.
+ * @param ignoreRdfSubjectAndObject if true, rdf:subject and rdf:object will be ignored. This is necessary when checking


So this ignores reifications (link values) referring to the IRI of the resource to be deleted if set to true?

Wouldn't this mean that a resource is referenced and hence cannot be deleted?

I would expect that the IRI can be used as an rdf:subject (outgoing link) but not as an rdf:object (incoming link) (https://docs.knora.org/paradox/02-knora-ontologies/knora-base.html#linkvalue).

Suppose we want to erase resource A, and resource B has a link value that points to A, and the link value is marked as deleted. The link value will still contain an rdf:object pointing to A. It's OK to ignore it, because B no longer refers to A.

But the link value would have an rdf:object with the IRI of A that does not occur anymore as a subject in the triplestore, right?

@tobiasschweizer

But the link value would have an rdf:object with the IRI of A that does not occur anymore as a subject in the triplestore, right?

That's correct. This wouldn't be a problem for Knora or for the GraphDB consistency checking. @lrosenth are you saying that these (marked-as-deleted) references would need to be erased for legal reasons?

tobiasschweizer · 2019-09-11T08:14:26Z

Since this is a killer feature in the proper sense, I would like to discuss this together with @benjamingeer and @lrosenth.

subotic · 2019-09-11T08:44:18Z

We could always create placeholder resources for deleted resources with incoming links.

benjamingeer · 2019-09-11T08:47:07Z

We could always create placeholder resources for deleted resources with incoming links.

The use case that @lrosenth had in mind is that he's creating a new project by doing an import, and he wants to remove some incorrect resources and import them again.

subotic · 2019-09-11T09:59:58Z

I know, but what if project A creates a resource and project B links to it. So now project A wants to delete the resource. Are we not going to allow this? The data belongs to project A and they should be able to do what they want IMHO.

benjamingeer · 2019-09-11T10:06:02Z

I know, but what if project A creates a resource and project B links to it. So now project A wants to delete the resource. Are we not going to allow this? The data belongs to project A and they should be able to do what they want IMHO.

Currently project A can mark the resource as deleted, but they can't erase it. I guess in this case, if they want to erase it, it would make sense to replace it with a dummy resource. @lrosenth ?

benjamingeer · 2019-09-11T10:07:52Z

Although the problem would be that the placeholder resource would have to be of the correct class, which could have required cardinalities for links to other resources. So we could end up having to create a whole graph of dummy resources just to erase one resource. This is the same problem as the bulk import problem.

benjamingeer · 2019-09-11T10:13:52Z

Since erasing resources in production is supposed to be something very rare, and can only be done by a sysadmin or project admin, maybe it would be best to leave it up to the admin to decide how to deal with this scenario. If they want to create a graph of dummy resources and change existing links first before erasing, they could do that via the API.

lrosenth · 2019-09-11T10:28:27Z

OK – I would suggest the following rules:

if a resource is not referenced by another resource: erase it completely (including associated SIPI-Files!)
if the resource is referenced by another resource: You cannot erase it at all! The user has first to remove the links to it (that is, the links from other resources to the one to be deleted have to be marked as deleted).
if a resource has only "incoming" links marked as deleted to it, these (already marked as deleted) links have to be removed/erased also completely. If it's simpler to "downgrad" the resource to a "dummy resource" that sais "I?ve been deleted" that would be a solution too – whatever is easier to implement!

Case (3) is not as urgent as (1) and (2)

Erasing a resource is always a "last resort action" that needs admin privileges

benjamingeer · 2019-09-11T11:06:20Z

@lrosenth

if a resource is not referenced by another resource: erase it completely (including associated SIPI-Files!)

We have a use case, #1078, where the same Sipi files could be reused in different resources: "Use case: bulk import has been done, and needs to be redone because data has changed, but images have not changed."

That seems similar to your use case here. Then you would want to keep the Sipi files, right?

Also, when you mark a resource or value as deleted, Knora doesn't remove its Sipi files. Should it?

if a resource has only "incoming" links marked as deleted to it, these (already marked as deleted) links have to be removed/erased also completely.

That seems feasible.

tobiasschweizer · 2019-09-11T11:21:36Z

Currently Knora never deletes files from Sipi, and I think this is still the safest policy, because of use cases like #1078.

Once a resource with an attached file value gets erased/deleted, will the file in Sipi then be unaccessible because no permissions will be granted for an erased/deleted resource?

In other words, the file would still be in Sipi, but it would never be served.

benjamingeer · 2019-09-11T11:29:15Z

In other words, the file would still be in Sipi, but it would never be served.

It would never be served, but it could be used again to create another resource or value, which is the use case in #1078:

Use case: bulk import has been done, and needs to be redone because data has changed, but images have not changed.

tobiasschweizer · 2019-09-11T11:36:15Z

It would never be served, but it could be used again to create another resource or value, which is the use case in #1078:

Which permissions would be required to use an existing file? If the original resource was erased, the original permissions cannot be determined. I am thinking of a copyright protected information that required the erasure of a resource.

benjamingeer · 2019-09-11T11:49:21Z

Which permissions would be required to use an existing file? If the original resource was erased, the original permissions cannot be determined. I am thinking of a copyright protected information that required the erasure of a resource.

To reuse an existing file, you'd have to move it to Sipi's tmp directory first. Then you can create a new resource as if you had just uploaded the file to Sipi. No permissions will be checked.

If you don't want to reuse the file, you can delete it from Sipi's storage using rm.

tobiasschweizer · 2019-09-11T11:59:33Z

To reuse an existing file, you'd have to move it to Sipi's tmp directory first. Then you can create a new resource as if you had just uploaded the file to Sipi. No permissions will be checked.

If you don't want to reuse the file, you can delete it from Sipi's storage using rm.

So this would require write access to Sipi's file system. I think that's fine since you would need admin privileges to use the functionality added with this PR.

tobiasschweizer · 2019-09-11T12:00:59Z

This PR looks fine to me. I leave the final approval to @lrosenth

benjamingeer · 2019-09-11T12:01:45Z

This PR looks fine to me. I leave the final approval to @lrosenth

Thanks. I'm currently writing the code to erase marked-as-deleted link values.

tobiasschweizer · 2019-09-11T12:03:22Z

You are the Eraser ...

… erased.

benjamingeer · 2019-09-11T15:03:38Z

@lrosenth Is this OK now? It erases deleted link values pointing to the resource that's being erased. For the reasons explained above, it doesn't touch files in Sipi. Does that seem reasonable to you?

lrosenth · 2019-09-11T15:09:48Z

Seems very reasonable to me! Maybe at some point we need a "Purge" algorithm whoiich detect unreferenced SIPI files and allows the to be deleted... But that's not urgent!

lrosenth

Thanks a lot!

benjamingeer · 2019-09-11T15:52:40Z

Thanks for the review!

feat(api-v2): Erase a resource (ongoing).

dd31b22

benjamingeer self-assigned this Sep 4, 2019

benjamingeer mentioned this pull request Sep 4, 2019

Ben's PR history #571

Open

Benjamin Geer added 2 commits September 9, 2019 10:04

Merge branch 'develop' into wip/1421-erase-resource

e6b46b8

feat(api-v2): Erase a resource.

4decb36

benjamingeer added this to the 2019-09 milestone Sep 9, 2019

test(api-v2): Add E2E test for erasing a resource.

5d63278

benjamingeer requested review from tobiasschweizer and lrosenth September 9, 2019 16:02

benjamingeer added API/V2 enhancement improve existing code or new feature labels Sep 9, 2019

test: Add comment.

da96daa

Merge branch 'develop' into wip/1421-erase-resource

ccc32f5

tobiasschweizer reviewed Sep 11, 2019

View reviewed changes

Benjamin Geer added 2 commits September 11, 2019 14:35

feat(api-v2): Erase deleted link values referring to a resource being…

4373b75

… erased.

docs(api-v2): Clarify erasing a resource.

2baa304

lrosenth approved these changes Sep 11, 2019

View reviewed changes

benjamingeer merged commit fb7adba into develop Sep 11, 2019

benjamingeer deleted the wip/1421-erase-resource branch September 11, 2019 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api-v2): Erase a resource #1429

feat(api-v2): Erase a resource #1429

benjamingeer commented Sep 4, 2019 •

edited

Loading

tobiasschweizer commented Sep 9, 2019

benjamingeer commented Sep 10, 2019

tobiasschweizer commented Sep 10, 2019

benjamingeer commented Sep 10, 2019

benjamingeer commented Sep 10, 2019

tobiasschweizer Sep 11, 2019

benjamingeer Sep 11, 2019

tobiasschweizer Sep 11, 2019

tobiasschweizer Sep 11, 2019

tobiasschweizer Sep 11, 2019 •

edited

Loading

benjamingeer Sep 11, 2019 •

edited

Loading

tobiasschweizer Sep 11, 2019

benjamingeer Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

subotic commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

subotic commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

lrosenth commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019 •

edited

Loading

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

lrosenth commented Sep 11, 2019

lrosenth left a comment

benjamingeer commented Sep 11, 2019

feat(api-v2): Erase a resource #1429

feat(api-v2): Erase a resource #1429

Conversation

benjamingeer commented Sep 4, 2019 • edited Loading

tobiasschweizer commented Sep 9, 2019

benjamingeer commented Sep 10, 2019

tobiasschweizer commented Sep 10, 2019

benjamingeer commented Sep 10, 2019

benjamingeer commented Sep 10, 2019

tobiasschweizer Sep 11, 2019

Choose a reason for hiding this comment

benjamingeer Sep 11, 2019

Choose a reason for hiding this comment

tobiasschweizer Sep 11, 2019

Choose a reason for hiding this comment

tobiasschweizer Sep 11, 2019

Choose a reason for hiding this comment

tobiasschweizer Sep 11, 2019 • edited Loading

Choose a reason for hiding this comment

benjamingeer Sep 11, 2019 • edited Loading

Choose a reason for hiding this comment

tobiasschweizer Sep 11, 2019

Choose a reason for hiding this comment

benjamingeer Sep 11, 2019

Choose a reason for hiding this comment

tobiasschweizer commented Sep 11, 2019

subotic commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

subotic commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

lrosenth commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019 • edited Loading

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

tobiasschweizer commented Sep 11, 2019

benjamingeer commented Sep 11, 2019

lrosenth commented Sep 11, 2019

lrosenth left a comment

Choose a reason for hiding this comment

benjamingeer commented Sep 11, 2019

benjamingeer commented Sep 4, 2019 •

edited

Loading

tobiasschweizer Sep 11, 2019 •

edited

Loading

benjamingeer Sep 11, 2019 •

edited

Loading

tobiasschweizer commented Sep 11, 2019 •

edited

Loading