Better handle file upload

[ericovasconcelos]

Description
The procedure described on https://marmelab.com/react-admin/CreateEdit.html#altering-the-form-values-before-submitting is not possible to do using api-platform, because it encapsulate the dataProvider. So the access to the actions to be able to include a new header according to the data being sent is not possible without breaking the api-platform dataProvider code.

Example
To view the description on what I'm suggesting, please view https://marmelab.com/react-admin/CreateEdit.html#altering-the-form-values-before-

const dataProvider = {
    // ...
    create: (resource, params) => {
        const { **dataHeaders**, ...record } = params.data;
        const headers = new Headers({
            'Content-Type': 'application/json',
        });
        **if (dataHeaders) {       //  This can be opinated. If data contains headers array, automatically set new headers on DataProvider
            for(let i = 0; i < dataHeaders.length; i = i + 1 ) {
                headers.set(dataHeader.name,dataHeader.value);
            }         
        }**
        return httpClient(`${apiUrl}/${resource}`, {
            method: 'POST',
            body: JSON.stringify(record),
            headers,
        }).then(({ json }) => ({
            data: { ...record, id: json.id },
        }));
    },
}

[alanpoulain]

Hello, I think it's doable by wrapping yourself the hydra data provider and by adding the headers on the fetchHydra parameter, like it's done for the authentication: https://api-platform.com/docs/admin/authentication-support/

[ericovasconcelos]

Hello,

Yes, actually I was able to do it by wrapping over the fetchHydra as suggested. Thank you.

One question: the use-case I'm using is to make the front-end to a resource that requires multipart/form-data, even when not sending the file field itself. (A resource with a upload field https://api-platform.com/docs/core/file-upload/#uploading-to-an-existing-resource-with-its-fields).

Real-admin is smart enough to change to multipart/form-data when the file field is filled, but when the field is not filled, it falls back to json. This way, I'm overwriting the create function of DataProvider to verify if this is the case and then manually forcing the multipart:

const dataProvider = baseHydraDataProvider(entrypoint, fetchHydra, apiDocumentationParser, true);

const addMultipartOverrides = (dataProvider) => ({
  ...dataProvider,
  create(resource, params) {
    if (resource === 'invoices') {
      return createOperationWithMultipart(resource, params);
    } else {
      return dataProvider.create(resource, params);
    }
  }});

const createOperationWithMultipart = (resource, params) => {
  const record = params.data
  let form_data = new FormData();
  for (const name in record) {
    if (record[name].rawFile !== undefined) {
      form_data.append(name, record[name].rawFile)
    } else {
      form_data.append(name, record[name])
    }
  }
  const headers = new Headers({
    'Authorization': `Bearer ${localStorage.getItem("token")}`,
  });
  return baseFetchHydra(`${entrypoint}/${resource}`, {
    method: 'POST',
    body: form_data,
    headers,
    mode: 'cors',
  }).then(({ json }) => {
    return ({
      data: { ...record, id: json.id },
    })
  });
}

export default addMultipartOverrides(dataProvider);

Considering that the use-case described on 'uploading to an existing resource with its fields' in Api-Platform is quite common and it kind of force us to some adjustment on the the react-admin, like the one I did, do you think this is a good use-case to embed into the baseHydraDataProvider itself and pass in some specific trigger?

(e.g. could use transform props on SimpleForm to add a forceMultipart field into the data that could trigger the dataProvider to encode the data into formData)

[alanpoulain]

I understand your use case but I don't have an elegant way of doing it in mind (using a forceMultipart field is too much dirty IMHO). If you find a nice way I will consider it!

[ericovasconcelos]

Makes sense. Considering that by setting input_formats attribute in API resource already informs swagger and deserializer to only accept that (and in https://api-platform.com/docs/core/file-upload/#uploading-to-an-existing-resource-with-its-fields use case, enforce multipart), it would be nicer if it could also inform the react-admin (maybe by Hydra Docs) to also only use accepted content-type.

But I do not know how or if it is possible to do it.

[ericovasconcelos]

API-Platform API and Admin components make very straightforward when creating CRUD operations on a resource. The basic flow involves creating an entity with the entity properties on the API side and the Resource component on the Admin site. This works great for any property you want to add to your entity.

If you would like that your entity holds a file property along others, you should only have to add a new property on the API side and a component on the Admin side.

This is how it is supposed to work. But it won´t. You will need to prepare your API component to be able to handle it. Thankfully, the documentation handles it very clearly https://api-platform.com/docs/core/file-upload/#configuring-the-existing-resource-receiving-the-uploaded-file.

After you follow the docs and create the component on the Admin side, you will have a fully functional CRUD on the resource, right? Well, unfortunately not. The CREATE will work great WHEN you submit a file. But you will have two problems:

1 – when working on an UPDATE operation; 2 - when working on a CREATE (or UDPATE) operation but does not fill in the file field;

The first case will fail because the operation would be set to a PUT verb and the API Component is not configured to work with multi-part on PUT operation. This would rises a new issue because even if you choose to configure the PUT operation on the API component as you did on the POST, you will fall to a PHP restriction on file upload on PUT verb (https://www.php.net/manual/en/features.file-upload.put-method.php).

So, the workaround to solve the 1st situation would be to set, on the API component side, the PUT operation to accept the multi-part input format and to accept a POST verb instead of PUT default verb. Of course that would not comply to RESTfull verb, but it seams to be a PHP limitation.

This is how my operation configuration looked like after the configuration:

*     itemOperations={
 *         "get","delete","patch",
 *         "put"={
 *            "method"="POST",
 *            "controller" = EmptyController::class,
 *            "input_formats" = {
 *                  "multipart" = {"multipart/form-data"}
 *             },
 *             "status" = 200,
 *            "openapi_context" = {
 *               "summary"="Replaces the Book Resource",
 *               "status" = 200,
 *            }
 *         },

This workaround might be applicable by only updating the documentation. (I could submit a pull request, if acceptable.)

The problem in the second situation is that if the user does not fill in the file field on the client-side, the data that is delivered from the form to the DataProvider will not be filled in with the File object, and the Admin component will not change the content-type to a multi-part and instead will believe the form should be encoded to a JSON. The endpoint on the API component side will throw an error of not acceptable content type.

This is the use case we've been discussed previously on this case. I've managed to apply a workaround by using your suggestion and wrapping manually the dataProvider and forcing the content-type and verb for my resource.

But, giving a second thought, anytime you add a field in a Form on the Admin component side of an API-Platform app, you are already reducing your choices to a multi-part content type and POST verb. This is true because you know that is how API-Platform expects how to handle that. This is not the case on a vanilla react-admin app.

So a more elegant way would be that, by adding a field in, the ApiPlatform/admin automatically takes responsibility to make the changes of content-type and verb and you don't need to apply for any work around on the dataProvider manually.

I thought in two ways this could be done:

1) by making the baseHydraDataProvider somehow introspecting the resource operation to check for a FileInput field when submitting a CREATE or UPDATE operation. 2) by creating a new API-platform file input component (e.g. from '@api-platform/admin') that could send a trigger message to the dataProvider. The message informs that this operation has a file input form and must be converted to multi-part and POST, even if doesn't have a File object and is run on UPDATE operation. (For example, it could be done by also embedding some hidden field.)

I´m glad to help on trying to implement that but would like to know your thoughts if it is viable.

Just for the record, if someone faces the same situation, I´ve managed to solve by using the following code on the data provider:

const dataProvider = baseHydraDataProvider(entrypoint, fetchHydra, apiDocumentationParser, true);

const addMultipartOverrides = (dataProvider) => ({
  ...dataProvider,
  create(resource, params) {
    if (resource === 'books') {
      return operationWithMultipart('CREATE',resource, params);
    } else {
      return dataProvider.create(resource, params);
    }
  },
  update(resource, params) {
    if (resource === 'books') {
      return operationWithMultipart('UPDATE',resource, params);
    } else {
      return dataProvider.update(resource, params);
    }
  }
}

const operationWithMultipart = (operation,resource, params) => {
  let url = '';
  switch (operation) {
    default:
    case 'CREATE':
      url = `${entrypoint}/${resource}`;
      break;
    case 'UPDATE':

      url = `${baseurl}${params.id}`;
      break;
  }

  const record = params.data
  let form_data = new FormData();
  for (const name in record) {
    // eslint-disable-next-line
    if(record[name]){
      if (record[name].rawFile !== undefined) {
        form_data.append(name, record[name].rawFile)
      } else {
        form_data.append(name, record[name])
      }
    }
  }
  const headers = new Headers({
    'Authorization': `Bearer ${localStorage.getItem("token")}`,
  });
  return baseFetchHydra(url, {
    method: "POST",
    body: form_data,
    headers,
    mode: 'cors',
  }).then(({ json }) => {
    return ({data: { ...record, id: json.id }})
  }).catch(function (error) {
    return Promise.reject(error);
  });
}
export default addMultipartOverrides(dataProvider);

[alanpoulain]

For the admin part, I think the best way would be to know that there is a FileInput used, and to always send data in multipart in this case. But I'm not sure it's doable without relying on dirty tricks.

[ericovasconcelos]

Hi @alanpoulain , thank you very much for your response and considerations.

Giving a thought on how to not use a "dirty trick", I guess that maybe following the HTML specifications will help find a solution.

By reviewing the docs (https://www.w3.org/TR/html52/sec-forms.html#element-attrdef-submitbuttonelements-formmethod), the way that HTML expects to set the content type and http verb are by:

1) having the content attributes of 'enctype' (for the form tag) or 'formenctype' (for the submit button) for the content type. 2) having 'method' (for the form tag) or 'formmethod' (for the submit button tag) for the HTTP verb.

So I guess that if react components are generating those correctly, then all we need to make sure is that the dataProvider is also able to read those.

I know we will have more difficulty with the form tags. But maybe on the submit button, it might be easier to implement those attributes. The questions will be if the dataProvider would support those, but if not, maybe implementing a dataProvider to follow those attributes on the queries will only make use implement the HTML specification, and won't be a dirty patch.

I'm diving a little deeper into a few implementations, but would like to know if you consider that this might be a good approach to run away from the dirty tricks?

[alanpoulain]

Yes it could be a good approach. However I think the main issue is how to communicate this information to the React Admin data provider. In API Platform Admin we cannot control too much how React Admin is handling information and we need to use its API if we want to add specific behavior.

[ericovasconcelos]

Hi Alan, I opened an issue (https://github.com/marmelab/react-admin/issues/6399#issuecomment-871053993) on react-admin repo to suggest the implementation of the HTML specification on the interface to the data provider. As well explained in the response and also on the docs (https://marmelab.com/react-admin/Architecture.html#providers), they left it to the opinionated implementation of the data providers:

React-admin apps must integrate with existing backends, but there isn’t any standard way to do so (or rather, there are too many standards to do so, e.g. REST, GraphQL, SOAP for data access).

So I guess that to React-Admin point-of-view, it is expected that api-platform/admin have an opinionated implementation to comply with the REST API Platform backend. As a matter of fact, it is already doing it, considering the code on the dataProvider.js:

    return convertReactAdminDataToHydraData(apiResource, data).then((data) => {
      const values = Object.values(data);
      const containFile = (element) =>
        isPlainObject(element) &&
        Object.values(element).some((value) => value instanceof File);

      if (!values.some((value) => containFile(value))) {
        return JSON.stringify(data);
      }

The containFile method is the one that is currently making the verification over the data passed from the form. The problem with the current verification is that it lead the setting of the form enctype to a end user decision of selecting or not a file. I guess this is not the desired scenario as the endpoint is only talking 'multipart/form-data'. I guess that the programmer should have a way to set this form enctype as any HTML form.

So on my dataProvider override, I chose the formEnctype expression (to be HTML specification complient) and made an opinionated use of that word on the data passed from the form:

const isOperationMultipart = (params) => {
  return (params.data.formEnctype && params.data.formEnctype === 'multipart') ? true : false;
}

This would make possible to anyone that want´s to set the form enctype of a form element using API Platform dataProvider to only use the documented way of transform (https://marmelab.com/react-admin/CreateEdit.html#transform) the data before submit:

const transform = data => ({
    ...data, 
    formEnctype: 'multipart'
})

export const BookCreate = props =>(
  <Create {...props} transform={transform}>
... 

The dataProvider.test.js test for 'React Admin create upload file' is only covering the scenario where the end user selected the file to upload, and not when he does not (but could). Basically because the data provider information about the form in only based on the data he receives, so, I assume that the only way to cover is after and check over an opinionated data is passed.

It is important to remember that I was able (and anyone can to) to use the override the default behavior of dataProvider and provide a fix on their own, using the baseFetchHydra on their own. But I guess that this is a very general issue that could affect a lot of API Platform admin users.

const addDataProviderOverrides = (dataProvider) => ({
  ...dataProvider,
  create(resource, params) {
    if (isOperationMultipart(params)) {
      return operationWithMultipart('CREATE',resource, params);
    } else {
      return dataProvider.create(resource, params);
    }
  },

What do you think? ( We would also need to cover that the update operation must be set to the POST verb. But if you agree with the multipart part, then the dataProvider itself can verify if the body is multipart and automatically set the vert to POST instead of PUT, or even defining a formMethod reserved keyword to data. The second would give back the decision to the programmer to override the method of the REST (as we could on an HTML implementation).

[alanpoulain]

Hello Erico, Why not using the transform prop like you did, I think it's not too much a dirty trick :wink: However ideally, it should not be the dev responsibility to do it. The CreateGuesser / EditGuesser component could detect if a FileInput child is used and automatically add the formEnctype if it's the case, WDYT? Converting a PUT to a POST seems necessary for PHP (but it's really bad in a REST PoV).

[ericovasconcelos]

Hi Alan, thanks for your reply.

I´m glad that you think the transform prop would be a good way to send the info. I guess that it would be the closest approach to HTML we would get with react-admin arch PoV (that I could find, at least).

I guess that implement the CreateGuesser / EditGuesser to detect and automatically add the formEnctype would be very nice. Because the implementation would just work out-of-the-box and very straightforward to a new resource.

I understand and agree with the PUT/POST consideration. It is very sorry to have this limitation to implement a clean REST, but as far as I looked it seams a general issue with PHP, right?

Well, I would be glad to help if you could use my help. Just tell me the best way I could help.

On the options I imagined, I could:

• write some tests on the dataProvider test to verify the formEnctype is set and if the response is in accordance with the specified value (similar to dataProvider.test.js:187);
• implement a version of the containFile function that verifies for the formEnctype and return true if is set to multipart (dataProvider.js:179);

Regarding the CreateGuesser / EditGuesser implementation, I would need to dive a little deeper as I didn´t yet.

Just tell me if and how you could use my help and I would be glad to send a pull request.

[alanpoulain]

You can try to make the PR with the transform prop and I will then make a PR to "guess" it automatically, if it's possible.

[ericovasconcelos]

Perfect. I´ll do that.

[ericovasconcelos]

Hi @alanpoulain,

I´ve submitted your review pull request #389.

I´ve added a test to check for the scenario where the data has a formEnctype set to multipart and doesn´t send a File object. Then verify if the body is from a type of FormData. The tests are all green and didn´t break other tests.

I understand it doesn´t break BC because formEnctype is not required. So if the user passes a File it will convert to multipart the same way it worked before, so no action is required for current users.

[ericovasconcelos]

Included support to the update operation. Available for your review on pull request #389. Best regards.