smartstore / SmartStoreNET

Open Source ASP.NET MVC Enterprise eCommerce Shopping Cart Solution
http://www.smartstore.com/en/net
GNU General Public License v3.0
2.64k stars 1.46k forks source link

WebApi: New MediaFiles endpoint example #2198

Closed TripleNico closed 3 years ago

TripleNico commented 3 years ago

From the release notes:

I conclude that you can now finally control all Media components via the WebApi. I see this also reflected in the Swagger UI However, when I try it on the Demo Website in Swagger, an exception occurs (as seen in the log section): Value cannot be null. Parameter name: mediaFile Also tested with 4.1.1 test environment and it doesn't seem to work properly there either.

My question to is how the MediaFiles endpoint can be used. ideally I'll would like to receive an example (based on Microsoft.OData.Client class) if that's possible?

TripleNico commented 3 years ago

Regarding this topic, i'am hitting an dead end and since ProductPicture etc.. isn't available in the WebApi v4 we're currently have no way to update our application to v4 and get it running, hopefully this can looked in to a.s.a.p.

In the meantime some additional info:

If you use the OData connected service from Microsoft for test then it fails to deserialise the JSON to the OData object. While if you check the RAW content in Fiddler then it seems all good. But i think there has been a mixed up in the metadata because the ~/odata/v1/MediaFiles is of type MediaFile:

{
  "@odata.context": "http://MySite/odata/v1/$metadata#MediaFiles/SmartStore.Core.Domain.Media.MediaFile",
  "value": [
    {
      "@odata.type": "#SmartStore.Core.Domain.Media.MediaFile",
      "FolderId": 2,
      "Name": "all-star-charcoal.jpg",
      "Alt": null,
      "Title": null,
      "Extension": "jpg",
      "MimeType": "image/jpeg",
      "MediaType": "image",
      "Size": 30074,
      "PixelSize": 48400,
      "Metadata": null,
      "Width": 220,
      "Height": 220,
      "CreatedOnUtc": "2019-11-06T07:58:45.177Z",
      "UpdatedOnUtc": "2021-02-10T14:20:19.67Z",
      "IsTransient": false,
      "MediaStorageId": null,
      "Id": 2
    }
  ]
}

While if you check the ~/odata/v1/$metadata is seems it is of type FileItemInfo? So my guess is that the deserialiser is trying to deserialise something to the wrong object because it would expect object of type MediaFile ?

<EntitySet Name="MediaFiles" EntityType="SmartStore.WebApi.Models.OData.Media.FileItemInfo"/>

Also only the endpoint ~/MediaFiles and ~/MediaFolders use the "@odata.type" property

mgesing commented 3 years ago

since ProductPicture etc.. isn't available in the WebApi v4...

I can see ProductPictures endpoints. What exactly is not available?

TripleNico commented 3 years ago

since ProductPicture etc.. isn't available in the WebApi v4...

I can see ProductPictures endpoints. What exactly is not available?

Sorry my bad, i mean endpoint ~/Picture this is not available anymore. In v3 i used endpoint ~/Picture to removed them if a category/brand/product is deleted.

For product pictures i used in v3 endpoint '~/api/v1/Uploads/ProductImages' and for manufacture and category pictures i used the Import Profiles to manage this. With v4 i was hoping to convert it al to use the '~/MediaFiles' endpoint for GET/POST/PATCH pictures of products/manufacturers/categories. But it seems that with the Visual Studio extension OData Connected Service the deserialization of 'MediaFiles' and 'MediaFolders' result in an exception:

InvalidOperationException: The response payload is a not a valid response payload. Please make sure that the top level element is a valid Atom or JSON element or belongs to 'http://docs.oasis-open.org/odata/ns/data' namespace.

Checking the raw response data is seems correct:

HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Type: application/json; charset=utf-8; odata.metadata=minimal
Expires: -1
Server: Microsoft-IIS/10.0
SmartStore-Net-Api-AppVersion: 4.1.1.0
SmartStore-Net-Api-Version: 1 4.1.1
SmartStore-Net-Api-MaxTop: 99999
SmartStore-Net-Api-Date: 2021-02-23T07:26:18.1898007Z
SmartStore-Net-Api-CustomerId: 5569
OData-Version: 4.0
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Tue, 23 Feb 2021 07:26:17 GMT
Content-Length: 547

{
  "@odata.context":"http://MySite/odata/v1/$metadata#MediaFiles/SmartStore.Core.Domain.Media.MediaFile","value":[
    {
      "@odata.type":"#SmartStore.Core.Domain.Media.MediaFile","FolderId":2,"Name":"all-star-charcoal.jpg","Alt":null,"Title":null,"Extension":"jpg","MimeType":"image/jpeg","MediaType":"image","Size":30074,"PixelSize":48400,"Metadata":null,"Width":220,"Height":220,"CreatedOnUtc":"2019-11-06T07:58:45.177Z","UpdatedOnUtc":"2021-02-10T14:20:19.67Z","IsTransient":false,"MediaStorageId":null,"Id":2
    }
  ]
}

I think this is because what i wrote earlier, the @odata.type describes type MediaFile but the OData MetaData describes type FileItemInfo

mgesing commented 3 years ago

'Picture' was renamed to 'MediaFile' and extended in the process of the media manager, so it's still available. GET /MediaFiles(123) and GET /MediaFiles works for me. Raw POST, PUT and PATCH are forbidden because there are further endpoints for it like SaveFile.

So this probably seems to be an issue with your auto-generated code or how the OData service is configured in your client.

There is not much I can contribute here because I've never used or wrote such an OData Connected Service client. I have created a VS 2019 preview console app and successfully included a connection to my OData service using both OData Connected Service or Unchase OData Connected Service. Anyway, the auto-generated code compiles without errors. But to connect to my OData service, I would have to further investigate this now, write the client, include API authorization....

TripleNico commented 3 years ago

Thanks for the extra info. Will debug some more with the OData Connected Service Client to see where the problem lays. Generating and compiling is indeed not the problem but when executing the Query then upon receiving the response the deserialisation fails. Tip: if you plan to build your own app for testing purpose then you can use the OData Client event SendingRequest2 to handle the authorisation. If you want a jump start i can send you some code.

Great that you further explained how the endpoints are now working, i didn't understand this by only looking at the Swagger documentation. I know this is on the roadmap to update.

Just to get thing clear few use cases:

  1. Query GET /MediaFiles see if picture exists
  2. If not call extension .SaveFile
  3. If it exists, check if the image is the same (can have the same filename) by comparing properties Size, CreatedOnUtc and UpdatedOnUtc of object type FileItemInfo If it's not equal then call extension .SaveFile
  4. If a Product/Manufacturer/Category is removed via the WebApi call POST /MediaFiles(123)/DeleteFile

Are these steps correct and most efficient?

mgesing commented 3 years ago

SaveFile is for uploading media files. It has an optional parameter "DuplicateFileHandling" that can handle duplicate files (passed as a ContentDisposition header together with multipart file data). Uploading files was not possible using OData endpoints before v.4., so I'm asking myself what your client actually did here?

Product, category and manufacturer are soft-deletable entities. They are marked as deleted but not physically removed from database. So you don't need to delete an image or media file associated with one of these entities when it's deleted. You actually should not do this because since version 4.0 media files can be assigned to multiple entities. You also do not need to delete the assignment of the image to a product because the product entity is soft-deletable. In other words, if you delete a product, category or manufacturer via API you don't need to do anything else. The backend does nothing else here either.

TripleNico commented 3 years ago

I didn't found the SaveFile extension being used in the Client example app you provide is that correct? Also why is there an extension SaveFile while OData uses SetSaveStream for this?

The reason i queried all those endpoints before is determine if an image exists and if it needs to be updated or deleted. In all times i want to prevent uploading all images since our Sync application is running every hour to sync SmartStore with our CRM system. Meaning handling products/categories/manufacturers/customers/customerroles/adresses/DependingPrices/orders/etc.. every hour would then also causes all images (few thousand) to being uploaded.

Regarding the deletion thanks for making that clear to me. In OData v3 i noticed that this wasn't always the case. Are the indexes than also updated? Meaning a product isn't visible anymore in a category, etc... Or it is simpler and can i assume when i DELETE an entity that this acts the same way as if a user in the backend clicks on Remove?

Could you also explain that Entity /MediaFile is of type FileItemInfo but if i GET the collection it returns type op MediaFile?

If you find the time it would be great if you can try it for yourself using the OData Connected Service to test if all is compliant. That would help me out a lot! ;-)

mgesing commented 3 years ago

As far as I remember, all endpoints can be tested by our API client. There is an "open file" button to select an image being uploaded.

SetSaveStream does not tell me anything. I cannot find any Microsoft.OData.Client dependency in our code.

There is also a FileExists method.

Do you mean search index? Yes, when a product is deleted, MegaSearch is notified via a hook and takes this into account during the next scheduled indexing.

If an entity is soft-deletable (aka ISoftDeletable) like product, category, manufacturer then it's never physically deleted. No matter if one of these entities is deleted via API or backend, the result is the same because the same service method is executed. If you physically delete a product, you will get problems in the form of inconsistent data (e.g. order items).

FileItemInfo is an API specific model acting as a wrapper class that contains MediaFile by property "File" but also some other properties. The purpose is to return additional data not contained in MediaFile at the same time, without risking collision when the core is changed.

TripleNico commented 3 years ago

Thanks for confirming that your application is working. That pointed me in the right direction regarding Uploading files. I ended up by remodelling the currently used endpoint POST ~/api/Uploads/ProductImages to work with the new POST ~/odata/v1/MediaFiles/SaveFile endpoint. This seems to be working except i have to determine the path:

Invalid path 'catalog'. Valid path expression is: {albumName}[/subfolders]/{fileName}.{extension}\r\nParameter name: path

Question here s what are the paths for Product, Category and Manufacturer? The path catalog was just a try. Additional, when a file is uploaded what would be the correct way to link is to an object like Product?

I still have to figure out what is going wrong with the Deserialization of GET ~/MediaFiles in order to determine file exists and file changed, etc...

Regarding the Delete, thanks for clearing that. That would save some extra code so i will have a cleanup of my code removing all additional code that comes after DELETE of a object. Since it is soft-deletable wouldn't that cluther the database overtime? I can understand that items that are linked to Orders isn't possible but how about a Product that was never purchased/used?

FileItemInfo is an API specific model acting as a wrapper class that contains MediaFile by property "File" but also some other properties. The purpose is to return additional data not contained in MediaFile at the same time, without risking collision when the core is changed.

Alright this makes sense but it doesn't explain why GET ~/MediaFiles returns the type MediaFile instead of FileItemInfo? Response i get:

{
  "@odata.context": "http://MySite/odata/v1/$metadata#MediaFiles/SmartStore.Core.Domain.Media.MediaFile",
  "value": [
    {
      "@odata.type": "#SmartStore.Core.Domain.Media.MediaFile",
      "FolderId": 2,
      "Name": "all-star-charcoal.jpg",
      "Alt": null,
      "Title": null,
      "Extension": "jpg",
      "MimeType": "image/jpeg",
      "MediaType": "image",
      "Size": 30074,
      "PixelSize": 48400,
      "Metadata": null,
      "Width": 220,
      "Height": 220,
      "CreatedOnUtc": "2019-11-06T07:58:45.177Z",
      "UpdatedOnUtc": "2021-02-10T14:20:19.67Z",
      "IsTransient": false,
      "MediaStorageId": null,
      "Id": 2
    }
  ]
}

Also of all endpoints in OData API there is no @odata.type except for the new endpoints MediaFiles and MediaFolders

EDIT: I think i found the reason whats going "wrong" with ~ /MediaFiles (same applies to MediaFolder) your description of object FileItemInfo triggered me to do some digging. It turns out (as you explained) that FileItemInfo is a wrapper class and that's exactly what's missing in the response of GET ~/MediaFiles. For example, if i Upload a file to endpoint POST ~/odata/v1/MediaFiles/SaveFile it returns the object 'FileItemInfo' on which you can see that MediaFile is added to FileItemInfo.File see response below:

{
  "@odata.context": "http://MySite/odata/v1/$metadata#MediaFiles/$entity",
  "Id": 15312,
  "Directory": "catalog",
  "Path": "catalog/2.jpg",
  "Url": "/media/15312/catalog/2.jpg",
  "ThumbUrl": "/media/15312/catalog/2.jpg?size=256",
  "File": {
    "FolderId": 1,
    "Name": "2.jpg",
    "Alt": null,
    "Title": null,
    "Extension": "jpg",
    "MimeType": "image/jpeg",
    "MediaType": "image",
    "Size": 1317255,
    "PixelSize": 2073600,
    "Metadata": null,
    "Width": 1920,
    "Height": 1080,
    "CreatedOnUtc": "2021-02-25T09:59:49.3028721Z",
    "UpdatedOnUtc": "2021-02-25T09:59:49.3028721Z",
    "IsTransient": true,
    "MediaStorageId": null,
    "Id": 15312
  }
}

Since MediaFiles endpoint is of type 'FileItemInfo' (According to the MetaData XML) i would expect the same response instead it only returns MediaFile missing it's rapper class FileItemInfo which also explains why my deserialisation is going wrong. So this is the current response:

{
  "@odata.context": "http://MySite/odata/v1/$metadata#MediaFiles/SmartStore.Core.Domain.Media.MediaFile",
  "value": [
    {
      "@odata.type": "#SmartStore.Core.Domain.Media.MediaFile",
      "FolderId": 1,
      "Name": "2.jpg",
      "Alt": null,
      "Title": null,
      "Extension": "jpg",
      "MimeType": "image/jpeg",
      "MediaType": "image",
      "Size": 1317255,
      "PixelSize": 2073600,
      "Metadata": null,
      "Width": 1920,
      "Height": 1080,
      "CreatedOnUtc": "2021-02-25T09:59:49.3028721Z",
      "UpdatedOnUtc": "2021-02-25T09:59:49.3028721Z",
      "IsTransient": true,
      "MediaStorageId": null,
      "Id": 15312
    }
  ]
}

What i would expect is:

{
   "@odata.context":"http://MySite/odata/v1/$metadata#MediaFiles/SmartStore.Core.Domain.Media.MediaFile",
   "value":[
      {
         "Id":15312,
         "Directory":"catalog",
         "Path":"catalog/2.jpg",
         "Url":"/media/15312/catalog/2.jpg",
         "ThumbUrl":"/media/15312/catalog/2.jpg?size=256",
         "File":{
            "@odata.type":"#SmartStore.Core.Domain.Media.MediaFile",
            "FolderId":1,
            "Name":"2.jpg",
            "Alt":null,
            "Title":null,
            "Extension":"jpg",
            "MimeType":"image/jpeg",
            "MediaType":"image",
            "Size":1317255,
            "PixelSize":2073600,
            "Metadata":null,
            "Width":1920,
            "Height":1080,
            "CreatedOnUtc":"2021-02-25T09:59:49.3028721Z",
            "UpdatedOnUtc":"2021-02-25T09:59:49.3028721Z",
            "IsTransient":true,
            "MediaStorageId":null,
            "Id":15312
         }
      }
   ]
}

EDIT extra: it seems to be a bug in MediaFilesController because GET ~/MediaFiles(Id) does return FileItemInfo object. But the Get without ID returns MediaFile

As you can see the private Convert is called:

        // GET /MediaFiles(123)
        [WebApiQueryable]
        [WebApiAuthenticate]
        public IHttpActionResult Get(int key)
        {
            var file = Service.GetFileById(key, _defaultLoadFlags);

            return Ok(Convert(file));
        }

But is not called here:

        // GET /MediaFiles
        [WebApiQueryable]
        [WebApiAuthenticate]
        public IHttpActionResult Get()
        {
            return Ok(GetEntitySet());
        }

So either correct this or change the MetaData saying it will return MediaFile instead of FileItemInfo

Thanks!

TripleNico commented 3 years ago

Additional questions/requests:

  1. I see currently multiple files in one upload is not supported, will this be added? Isn't it just easy by adding a For int loop and then returning the result as list/IEnumerable/IQueryable/etcc?
  2. Is it possible to add the linking of objects? For example, add SKU in content-disposition so after upload SmartStore links it to Product with SKU XX. Same for Category and Manufacture? Else i would have to do upload of a file in two steps. Step 1: Upload the file. Step 2: Find the new File and link the ID to a Product, Category or Manufacturer
  3. If step 2 is possible it would be great if the DisplayOrder in case of Product would also be excepted.
mgesing commented 3 years ago

Path is always: catalog/<file name> (e.g. catalog/my-image.jpg) for catalog related media files. You can see the path in the right panel of the Media-Manager when selecting an image.

Yes, at some point the database will become very large if soft-deletable entities are deleted again and again, but at some point we will also have a trash function for this.

GET /MediaFiles returns the entity instead of FileItemInfo to support all ODataQueryOptions when querying lists of entities. Maybe your auto-generated code generator can't make the distinction between these two models based on the metadata. Could you please test and replace the code of the GET /MediaFiles API endpoint by following code:

[WebApiQueryable]
[WebApiAuthenticate]
public IHttpActionResult Get(ODataQueryOptions<MediaFile> queryOptions)
{
    IQueryable<MediaFile> query = null;

    var hasClientPaging = Request?.RequestUri?.Query?.Contains("$top=") ?? false;
    if (!hasClientPaging)
    {
        var maxTop = WebApiCachingControllingData.Data().MaxTop;
        var top = Math.Min(this.GetQueryStringValue("$top", maxTop), maxTop);

        query = queryOptions.ApplyTo(GetEntitySet(), new ODataQuerySettings { PageSize = top }) as IQueryable<MediaFile>;
    }
    else
    {
        query = queryOptions.ApplyTo(GetEntitySet()) as IQueryable<MediaFile>;
    }

    var files = query.ToList();
    var result = files.Select(x => Convert(Service.ConvertMediaFile(x)));

    return Ok(result);
}

Does your client work with this? It may have a disadvantage because I am not sure whether all ODataQueryOptions are really taken into account.

Question 1, 2, 3: No, not for MediaFiles endpoints. They are meant to map the media infrastructure one-to-one, no custom catalog stuff here. By the way, for uploading product images there is an older, non-OData, generic multipart REST endpoint POST /api/v1/Uploads/ProductImages that's handling multiple uploads including product media file assignments. It has also a duplicate file handling, and you also also send a picture ID aka Media File ID to sync existing images, if you know it. The DisplayOrder is simply increased here by one for each image uploaded per product. Docu is here but it only works with product images, not for category or manufacturer images.

TripleNico commented 3 years ago

Path is always: catalog/<file name> (e.g. catalog/my-image.jpg) for catalog related media files. You can see the path in the right panel of the Media-Manager when selecting an image.

Thanks got it! Also the tip for looking in the Media-Manager is very helpful! Is saw in the marketplace that there is now also a plugin for documents etc.. at products. Would the folder then just be FileManager? And is there also an extra API endpoint for mapping MediaFile to the product then?

Yes, at some point the database will become very large if soft-deletable entities are deleted again and again, but at some point we will also have a trash function for this.

That would be great (and necessary), keeping things clean which off course benefits the performance!

GET /MediaFiles returns the entity instead of FileItemInfo to support all ODataQueryOptions when querying lists of entities. Maybe your auto-generated code generator can't make the distinction between these two models based on the metadata.

That's understandable but it conflicts with the MetaData XML stating a different object. I guess the client code needs to be aware of dynamic types of some sort?

Could you please test and replace the code of the GET /MediaFiles API endpoint

That seems to do the trick and returns the right (expected) object. However things brings a new exception:

Microsoft.OData.ODataException: 'The Id cannot be computed, since the navigation source 'File' cannot be resolved to a known entity set from model.'

This was the RAW response:

HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Type: application/json; odata.metadata=minimal; charset=utf-8
Expires: -1
Server: Microsoft-IIS/10.0
SmartStore-Net-Api-AppVersion: 4.1.1.0
SmartStore-Net-Api-Version: 1 4.1.1
SmartStore-Net-Api-MaxTop: 120
SmartStore-Net-Api-Date: 2021-02-26T08:16:15.0498239Z
SmartStore-Net-Api-CustomerId: 5569
OData-Version: 4.0
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Fri, 26 Feb 2021 08:16:15 GMT
Content-Length: 655

{
  "@odata.context": "http://MySite/odata/v1/$metadata#MediaFiles",
  "value": [
    {
      "Id": 2,
      "Directory": "content",
      "Path": "content/all-star-charcoal.jpg",
      "Url": "/media/2/content/all-star-charcoal.jpg",
      "ThumbUrl": "/media/2/content/all-star-charcoal.jpg?size=256",
      "File": {
        "FolderId": 2,
        "Name": "all-star-charcoal.jpg",
        "Alt": null,
        "Title": null,
        "Extension": "jpg",
        "MimeType": "image/jpeg",
        "MediaType": "image",
        "Size": 30074,
        "PixelSize": 48400,
        "Metadata": null,
        "Width": 220,
        "Height": 220,
        "CreatedOnUtc": "2019-11-06T07:58:45.177Z",
        "UpdatedOnUtc": "2021-02-10T14:20:19.67Z",
        "IsTransient": false,
        "MediaStorageId": null,
        "Id": 2
      }
    }
  ]
}

Question 1, 2, 3: No, not for MediaFiles endpoints. They are meant to map the media infrastructure one-to-one, no custom catalog stuff here. By the way, for uploading product images there is an older, non-OData, generic multipart REST endpoint POST /api/v1/Uploads/ProductImages that's handling multiple uploads including product media file assignments. It has also a duplicate file handling, and you also also send a picture ID aka Media File ID to sync existing images, if you know it. The DisplayOrder is simply increased here by one for each image uploaded per product. Docu is here but it only works with product images, not for category or manufacturer images.

I was aware and using the Uploads/ProductsImages with v3. But now i can use one endpoint for all MediaFiles i've changed the approach to create one function that handles the MediaFiles of Product/Category/Manufacturer/Etc...

mgesing commented 3 years ago

Names of the default folders are located here. New (sub-)folders can be created via POST /MediaFolders/CreateFolder {"Path":"content/my-folder"}, link.

Media files can be assigned to products by POST /Products(<Product.Id>)/ProductPictures(<MediaFile.Id>), link. The body is ProductMediaFile content.

Please attach the code of your client here so I can take a closer look at the error message and to ensure that any changes do not create new errors.

TripleNico commented 3 years ago

Names of the default folders are located here. New (sub-)folders can be created via POST /MediaFolders/CreateFolder {"Path":"content/my-folder"}, link.

Media files can be assigned to products by POST /Products(<Product.Id>)/ProductPictures(<MediaFile.Id>), link. The body is ProductMediaFile content.

Great, thanks for the info!

Please attach the code of your client here so I can take a closer look at the error message and to ensure that any changes do not create new errors.

Sure, what would you like to see (since i have to strip down some code)? Or should i create a little sample project for you?

mgesing commented 3 years ago

Code or sample project where I can reproduce the error.

TripleNico commented 3 years ago

Code or sample project where I can reproduce the error.

I've created a sample project for you using Microsoft OData Connected Service. If correctly you should have a Github invite to the private repository.

It uses the API from your Demo website. Just download and run it. Please note that you first have to change the Get ~/MediaFiles in the webapi to this https://github.com/smartstore/SmartStoreNET/issues/2198#issuecomment-786036554 before it works. Or just adjust the API link and keys to your development one.

mgesing commented 3 years ago

I've updated the API configuration that make MediaFiles and MediaFolders work with your sample code. It returns IEnumerable of MediaFile again (not MediaFileInfo), so you have to update your API plugin and your OData connected service.

But.... the OData Connected Service generally creates wrong code for OData actions and functions. Examples: it incorrectly generates FileItemInfoSingle instead of IEnumerable as return value for GET /MediaFiles/GetFilesByIds(Ids=[1,2,3]) and it often uses the wrong HTTP method (GET instead of POST) which will result in a 404...

We have to check, re-configure and test all OData actions and functions with regard to their configuration to see whether an auto-generated code generator translates it correctly. This is really a lot of work to do and will take its time. We'll have to do this later. I have created an issue for it.

If I were you, I would switch to generic API requests like in the Smartstore API client to get back more control over client code.

TripleNico commented 3 years ago

Due to your update all seems to be working now, thanks!

I can understand that checking the API with the MSFT OData Client code generator would be alot of work. It would though be an extra plus for other customers out there than want to start coding real quickly with your WebApi.

I do have a question though the parameters like DuplicateFileHandling should they have their own form-data like this:

-----------9FB32361-6F89-49D8-B52C-1F4B4C4D5761
Content-Disposition: form-data; name="Id"

14016
-----------9FB32361-6F89-49D8-B52C-1F4B4C4D5761
Content-Disposition: form-data; name="file"; PictureId="14016"; Path="content\14210_1.jpg"; DuplicateFileHandling="Overwrite"; filename="14210_1.jpg"
Content-Type: image/jpeg

or should it be combined with the file info like this:

-----------9FB32361-6F89-49D8-B52C-1F4B4C4D5761
Content-Disposition: form-data; name="file"; Id="14016"; Path="catalog\14210_1.jpg"; DuplicateFileHandling="Overwrite"; filename="14210_1.jpg"
Content-Type: image/jpeg
mgesing commented 3 years ago

For multipart content it's the second one, Content-Disposition parameter on the file content part, because it expects exact one content part here. Otherwise it's a JSON property in the posted content:

POST ~/MediaFiles(14332)/MoveFile 
{"DestinationFileName":"catalog/rename-me.jpg","DuplicateFileHandling":"Rename"}
TripleNico commented 3 years ago

Great, than i will add it to the form-data of the file itself. Although this raises another question: If DuplicateFileHandling is Overwrite on SaveFile should the Identifier described as Id be added to the file form-data or to it's own form-data? In other words should it be this:

-----------9FB32361-6F89-49D8-B52C-1F4B4C4D5761
Content-Disposition: form-data; name="Id"

14016
-----------9FB32361-6F89-49D8-B52C-1F4B4C4D5761
Content-Disposition: form-data; name="file"; Path="catalog\14210_1.jpg"; DuplicateFileHandling="Overwrite"; filename="14210_1.jpg"
Content-Type: image/jpeg

or should it be like this:

-----------9FB32361-6F89-49D8-B52C-1F4B4C4D5761
Content-Disposition: form-data; name="file"; Id="14016"; Path="content\14210_1.jpg"; DuplicateFileHandling="Overwrite"; filename="14210_1.jpg"
Content-Type: image/jpeg
mgesing commented 3 years ago

As mentioned, SaveFile is limited to exactly one multipart so the parameters (Path, IsTransient, DuplicateFileHandling) must be passed through that multipart.

In principle, these methods do nothing more than collect data and parameters and use it to call the corresponding MediaService method. The duplicate checks are made by file path here, not by ID. Details can be viewed in method CheckUniqueFileName.

TripleNico commented 3 years ago

Aha, that explains the current behaviour i'm seeing. So overwriting a file with a different file name results not in a overwrite but in a new file, correct?

For example filename 1234-1.jpg is not the same as 1234_1.jpg correct? If so if one want to use the Overwrite function the filenames have to match else it won't be overwritten?

Additional what would be the approach to Overwrite a jpg with png? For example the first of four images of a product get's changed by a png. Then you want to Overwrite the first image of that product. But it's a different filename (because of the extension) so i guess this than creates a new file if i'm not mistaking?

mgesing commented 3 years ago

So overwriting a file with a different file name results not in a overwrite but in a new file, correct?

Yes, if no file exists with this name.

For example filename 1234-1.jpg is not the same as 1234_1.jpg correct?

Yes.

If so if one want to use the Overwrite function the filenames have to match else it won't be overwritten?"

Yes, the names have to match.

Additional what would be the approach to Overwrite a jpg with png?

MoveFile?

TripleNico commented 3 years ago

MoveFile?

Alright so use MoveFile to Upload a new file that overwrites a existing file?

For example: Existing file is called 1234-1.jpg and the new png file is called 1234_1.png then i will call MoveFile with DuplicateFileHandling set to Overwrite, correct?

Or do i first have to use SaveFile for the upload of the new PNG and then call MoveFile to Overwrite the existing jpg? Wil this also inherit then the mapping to a product?

Or option 3 use SaveFile to upload and map it to a product and call DeleteFile afterwords on the old JPG?

muratcakir commented 3 years ago

You cannot change a file's MIME type during a move operation: https://github.com/smartstore/SmartStoreNET/blob/bb559b3aab2ff66175dbf8944e377a71d8e62373/src/Libraries/SmartStore.Services/Media/MediaService.cs#L898

Because, simply said, JPG is not PNG. Therefore option 3 is the way to go.

TripleNico commented 3 years ago

You cannot change a file's MIME type during a move operation:

https://github.com/smartstore/SmartStoreNET/blob/bb559b3aab2ff66175dbf8944e377a71d8e62373/src/Libraries/SmartStore.Services/Media/MediaService.cs#L898

Because, simply said, JPG is not PNG. Therefore option 3 is the way to go.

Thanks for confirming my hunch. Option 3 it is! I just realised that i can always call SaveFile for Overwrite because:

  1. If the filename is the same (that means also the extension) than the response body will contain the same Id as used for upload
  2. If the response body Id is not the same as one used for uploading then SmartStore has created a new MediaFile meaning we can Delete the previous one.

Just double checking, if i call DeleteFile will this remove also any mapping to objects?

mgesing commented 3 years ago

Yes, as long as you call it with the appropriate parameters: https://github.com/smartstore/SmartStoreNET/blob/bb559b3aab2ff66175dbf8944e377a71d8e62373/src/Plugins/SmartStore.WebApi/Controllers/OData/MediaFilesController.cs#L430-L431 https://github.com/smartstore/SmartStoreNET/blob/bb559b3aab2ff66175dbf8944e377a71d8e62373/src/Libraries/SmartStore.Services/Media/MediaService.cs#L483

TripleNico commented 3 years ago

Thanks, it seems i've got things up and running now. Although i noticed when i bulk(1000+) upload images for products then not all products get a thumbnail generated. If i scroll through some categories the thumbnails of the product are not showing but if i click on the product then all images are there and correct.

Can it be that the bulk upload is to fast? Or maybe the thumbnails are generated in batches and some are missed by this? I do want to test this if put in a sleep of let's say 1 second per upload if this fixes the issue.

Edit 1: Tested it with a sleep of 1 second between uploads and makes no difference. I don't know exactly how thumbnails are created after Upload but hopefully this is an easy fix or explainable.

Edit 2: If i inspect the URL i noticed that the MediaFileId is 0 for products that don't have a working thumbnail:

<img src="//localhost:3721/media/0/default-image.png?size=256" alt="Something" title="Something" loading="lazy">

Example of a working one:

<img src="//localhost:3721/media/23894/default-image.png?size=256" alt="Something" title="Something" loading="lazy">

So i started digging further. If i check the Product Picture backend i can see that the main image is set. In this case only one image is assigned to the product.

This is how the MediaFile table looks if i search for that specific picture: image

In my opinion this looks alright so next step is checking the ProductMediaFile table: image

Also this look fine, so it tested some more and it seems if the product only has one image assigned then the thumbnail lookup fails. Changing displayorder from 0 to 1 has no affect. There also doesn't seem to be a relation between a new or updated (OverWriten) image.

mgesing commented 3 years ago

Please ensure that Product.MainPictureId references the main picture of a product. It is responsible for displaying the preview image in for example product lists.

muratcakir commented 3 years ago

INFO: thumbnails are never created during upload, but lazily on first hit. Therefore: throttling upload won't fix anything.

TripleNico commented 3 years ago

Please ensure that Product.MainPictureId references the main picture of a product. It is responsible for displaying the preview image in for example product lists.

Thanks for notifying this. Never thought of it because if you upload a image for the first time then it is set by SmartStore self. But if you remove the first image afterwords, upload a new one than the old one isn't overwritten. Thanks to your hint i now have a function that checks after upload if the MainPictureId matches the first MediaFileId found in Product.ProductPictures if not then it will update it with the first one. This seems to resolve the issue.

INFO: thumbnails are never created during upload, but lazily on first hit. Therefore: throttling upload won't fix anything.

Aha, thanks for clarifying that. That confirms indeed why the extra sleep didn't do anything ;-)