relation_entity_delete() is unneccesarily destructive.

Good idea! Couple of things...

    foreach($relation->endpoints[LANGUAGE_NONE] as $key => $endpoint) {
      if($endpoint['entity_id'] == $eid) {
        unset($relation->endpoints[LANGUAGE_NONE][$key]);
      }
    }

This needs entity type condition as well, otherwise unnecessary endpoints might end up being removed. In addition to that the notification message needs to be rewritten too.

Thanks for patch, here's my quick review

• Line indentations are way off on some lines, 'if', 'foreach' etc.. should include space before opening parenthesis (coding standards).
• The rids of to be deleted relations are stored anyway so relation_delete_multiple can be used to delete all at once to reduce needed db operations.
• It's better to user term 'valid' instead of 'legal'
• This also needs tests.

+    foreach($relation->endpoints[LANGUAGE_NONE] as $key => $endpoint) {
+      if($endpoint['entity_id'] == $entity->eid 
+        && $endpoint['entity_type'] == $entity_type) {
+        unset($relation->endpoints[LANGUAGE_NONE][$key]);
+      }
+	}

• There is no $entity->eid

tests...

Marked as blocker for #1707058: Relation 7.x-1.0.

Here are the tests. I had to make some changes to original patch to make it work.

The tests_only patch above had wrong assert and thus it displays as passed. Other patch had it correctly and that's now committed.

As chx pointed out at irc, relation_load_multiple can be used to improve performance

Here is a patch that uses relation_load_multiple and just for those relations that we need to resave. A new query is in place to fetch relation arity.

I guess this needs benchmarking to figure out if we need limits and more complex processes to clean the data.

changing tags

There is no easy solution to this, otherwise it would've been applied to core and taxonomy term references already (#1281114: Database records not deleted for Term Reference Fields after Term is Deleted.

I prefer current functionality a lot and I think the way forward could be to just make it possible that when working with huge number of relations the API could be used without automatic cleanup and then do the cleanup some other way later on. Any thoughts on this?

pfft

Retroactive clean up could be a real pain though. What are you going to do, run though every row in the endpoints field data table and check and see if the entity still exists, and if not, delete it (remembering that that can't be done just in the DB, since each entity stores it's data in it's own table)? Or make a new table of "deleted entities"?

It might make sense to have some kind of per-relation-type option that decides whether relations should be cleaned up or not if an entity is delete, and they don't go below their minimum arity. That would work for the case in the original post, no? If a relation goes below it's minimum arity, then it's no longer valid, and should be deleted.

Also, because this is fairly edge-casey, I think it probably doesn't need to be a release blocker...

I agree this is not a blocker anymore.

I was thinking about using job_scheduler with deleted entity id stored there and that process could check all relations that have given endpoint and resave / delete them x at a time. But then, I don't like the idea of having dependency added to relation.