As stated on #1764462-3: Can't create file using services api. Exception in _uuid_services_entity_access()., files should never be updated or replaced. If needed, we should create a new file and delete the old file instead.

The Problem:

uuid_services is replacing all entity methods with custom functions that deal with uuid but files, IMHO, are a special case.

The problem is that uuid_services breaks the file resource, by removing the create operation and using the update operation instead, when actually files shouldn't be updated, and the only operations exposed by services are create, retrieve, delete, and index.

Proposed Solution:

Create custom wrappers for file entities, similar to the ones defined in file_resource.inc in the services module. These wrappers would use a custom acces function that will load the file by uuid and call _uuid_services_entity_access() and in general, wrap _file_resource_create(), _file_resource_delete(), _file_resource_retrieve() and _file_resource_index() to load the fid via uuid or add uuid information to the created file.

This would also solve #1741028: Support creation for files since we'd not need the creation callback for file entities anymore.

API Changes

The services for files would be different from other entity resources in uuid_services, but by using create instead of update, they would be more similar to the default file resource operations provided by services.

UI Changes

Operations for the file resource will be different in the ui and users who had selected the update operation for file resources would need to check the create operation.

Since the file resource is not working anyways, these two API changes shouldn't break anyone's code.

CommentFileSizeAuthor
#14 uuid_services-fix-file-create-1778484-14.patch7.88 KBalbertski
#7 uuid_services-allow_file_creation-1778484-7.patch7.66 KBsylus
#6 uuid_services-allow_file_creation-1778484-6.patch7.64 KBsylus
#2 uuid_services-allow_file_creation-1778484-2.patch7.63 KBlucascaro
#1 uuid_services-allow_file_creation-1778484-1.patch6.92 KBlucascaro

Comments

lucascaro’s picture

lucascaro commented
Status: Active » Needs review
StatusFileSize
new uuid_services-allow_file_creation-1778484-1.patch6.92 KB

Here's a starting patch that lets us create files using uuids.

Note that due to how services determines the controller, the create method shouldn't have a parameter so the creation of a new file using REST is performed by doing a GET request to endpoint/file and adding the uuids in the body:

{
"file":"/9j/4A[...]FAH/2Q==",
"filename": "filename.jpg",
"uuid": "xxxxxxxx-1007-6684-91f8-xxxxxxxxxxxx",
"type": "image"
}

Also note that this patch contains the patch from #1764468: Variable $entity_ids got renamed and needs to be updated. (one line) and it's not ready yet to be merged since it would need more testing.

I've tested creating a file using a REST server (create method) and also retrieving files

lucascaro’s picture

lucascaro commented
StatusFileSize
new uuid_services-allow_file_creation-1778484-2.patch7.63 KB

After more testing I realized I still had problems with duplicated uuids and assiging them to the file entitites, so here's a follow up patch that checks for existing files with the same uuid and replaces them. It might be worth to move this action back to the update (upsert) method since we have to ensure that there aren't duplicated uuids anyways, but I'll wait for more info from the maintainers before doing that.

With this patch we should be able to create new files and update old ones. The update will actually delete files with the same uuid and replace them with the new once. Not sure if this is the best approach but it makes sense to me so far since we're dealing with uuids.

This is also checking permissions for creating files loading the file by uuid and checking with _file_resource_access().

The main difference here is the file create function: 

 $uuid = $entity['uuid'];
  $file_id = entity_get_id_by_uuid('file', array($uuid));
  if (!empty($file_id)) {
    // Remove existing file if present using the services callback to ensure
    // uniqueness.
    foreach ($file_id as $id) {
      _file_resource_delete($id);
    }
  }
  $response = _file_resource_create($entity);
  if (!empty($response['fid'])) {
    // Set the uuid.
    $entities = entity_load('file', array($response['fid']));
    $entity = reset($entities);
    $entity->uuid = $uuid;
    entity_save('file', $entity);
    $response['uuid'] = $uuid;
    $response['uri'] = services_resource_uri(array('file', $uuid));
  }
  return $response;

As you see this where the existence of other files is checked and we delete them in order to replace them with the new one.

Feel free to talk to me in IRC if you have any questions.

Cheers.

succinct’s picture

succinct commented

I can't get this patch to apply against the most recent Dev release of UUID. I tried implementing the changes manually, and it did not seem to fix the issue.

dillix’s picture

dillix commented

I've just updated uuid_services manually, there is mistake in patch:

unset($resources[$entity_type]['create']);

should be:

unset($resources[$entity_type]['operations']['create']);

dillix’s picture

dillix commented
Status: Needs review » Reviewed & tested by the community

We tested #1, all works fine, but one line should be changed in it, as I wrote in #4.

sylus’s picture

sylus commented
StatusFileSize
new uuid_services-allow_file_creation-1778484-6.patch7.64 KB

Updated the patch for latest dev and works great!

sylus’s picture

sylus commented
StatusFileSize
new uuid_services-allow_file_creation-1778484-7.patch7.66 KB

updated patch for comment in #4:

sylus’s picture

sylus commented
Status: Reviewed & tested by the community » Needs review

Actually wait sadly this isn't working. I keep getting a DeployServiceException: Service Error: 404 Not Found: Could not find the controller in DeployServiceRest->httpRequest() line 70 of DeployServiceRest.inc.

Everything works excepts for files when deploying.

gilgabar’s picture

gilgabar commented
Status: Needs review » Needs work

Re: #8, deploy uses a PUT, i.e. an update, for everything so with this patch the update operation for file entities has been removed which results in the 404.

I agree that file entity handling is broken with uuid services, but the approach used here isn't the best way to deal with it. The update operation is actually the correct one to use. The services api operations naming scheme confuses things. A create operation is really the POST method and the update operation is really the PUT method. They don't actually map exactly to create and update. POST means create this resource and assign an id for me. PUT means create this resource at the id given and if something already exists there then replace it. So even though services is calling that operation 'update' it actually means the exact thing we want to do here with files.

Can you find any documentation that elaborates on the comment in services/resources/file_resource.inc about not updating files? I'm having trouble finding anything outside of that comment that says anything similar. The API seems to disagree with that statement, so it seems a bit confusing.
http://api.drupal.org/api/drupal/includes%21file.inc/function/file_save/7
http://api.drupal.org/api/drupal/includes!file.inc/function/file_save_da...

admataz’s picture

admataz commented

I agree this is confusing. I attempted to write a conditional to override the default PUT in Deploy for when the entity is of type file to make it a POST - just to see what would happen. Instead of a 404 I started getting out of memory errors and a status 500 when submitting the data via a test (not Deploy). So it seems it's not only the the http method - saving file entities via services is generally broken.

gilgabar’s picture

gilgabar commented

POST is the wrong thing to use, at least with UUID, as I mentioned in #9 above. PUT is exactly the right thing. So there isn't a lot to be gained venturing further in that direction.

There are two main problems I have seen with file handling via Deploy/UUID/Services. One is an access callback issue partially addressed in #1694972: File entity values fail on deployment. The other is a naive handling of file saves that doesn't address conflicting URIs. See #1969222: UUID Services: exceptions due to file entities with the same URI but different UUIDs for a fix. After patching both of those issues I have had no problems with files sent via deploy.

The patches in this issue attempt to address both of those problems, but I don't think the solutions are appropriate. I would recommend closing this and focusing efforts on the other more specific issues.

sylus’s picture

sylus commented

Thanks gilgabar you are correct and after following your posts deploy + files is working for me too. Thanks for taking the time to clarify.

lisa.rae’s picture

lisa.rae commented

Implementing the patches from #11 above did not rectify the issue at hand for me.

albertski’s picture

albertski commented
StatusFileSize
new uuid_services-fix-file-create-1778484-14.patch7.88 KB

I was not able to get the patch to work correctly so I made some fixes and now I am able to create files.

Also since we are using create we have to use POST instead of PUT. If PUT is the better route we should change it to update instead of create.

vbouchet’s picture

vbouchet
Location Toulouse
commented
Status: Needs work » Needs review

Change to needs review has the last patch has not been submitted for testing.