Don't send tags during upload if none are in options

[sarsonj]

Some S3 compatible storages don't support tags and upload fails, when tags are presented (even empty one). The example of this storage is Digital Ocean spaces, that we are using.

This small change sends tags only in case, when tags are presented in file options. It should be safe change, because it there should not have been difference between file uploaded with empty tags and no tags (IMHO). Also when somebody wants to use tags, he wants to use storage, that supports tags, so that it is fine, that upload fails (as fails now).

[dplewis]

Thank you for the contribution. Can you write a test case?

[mtrezza]

Some S3 compatible storages don't support tags and upload fails, when tags are presented (even empty one) ... It should be safe change, because it there should not have been difference between file uploaded with empty tags and no tags (IMHO).

I think this is a contradiction. "The change shouldn't make any difference, but it does make a difference for DigitalOcean." The counter argument could be that the adapter is just the intermediary that passes on whatever it gets. This change strips the developer of being able to define whether they want to send nothing at all (by not setting tag) or an empty object (tags:{}).

From #134:

it has no benefit to send empty tags dictionary to the S3.

Even though I agree that it probably shouldn't make a difference, I am not sure that is always the case. There may be a S3 compatible API where it does make a difference. Just like file upload shouldn't fail with DigitalOcean because of tags.

Wouldn't the issue be solved by not setting tags when uploading a file?

[sarsonj]

@mtrezza probably yes, but then the fix has to be done on Parse level.

From the other way, support for tags is on ParseFile - so that it should not be S3 specific feature only (even that it currently is). And on ParseFile level it means no tags -> tags are {} right?

Another option to fix this is to add configuration parameter to parse-server-s3-adapter (storageSupportsTags or so).

[mtrezza]

on ParseFile level it means no tags -> tags are {} right?

Would it make sense to rewrite the test case where you don't set a tag (not even an empty tag object {}) and expect that the S3 adapter does not send the tag parameter at all (not even an empty tag object {})? If I understood correctly, that seems to be the underlying basic issue.

[sarsonj]

@mtrezza The problem is, that Parse always calls createFile function with with options = {tags: {}, metadata: {}}, even when we don't specify any tags / metadata.

So yes, it can be fixed on Parse side as well, when there are no metadata / tags, they will not be send to files adapter in options. I didn't want to make change in Parse itself :-), but if this would better to fix it on that side, then I can look there.

[mtrezza]

Well, pragmatically speaking, it may be easier to do the change here, but I'm not sure that it would be the "cleanest" solution.

If the tags are only used to be passed on to the S3 adapter, then the change in Parse.File would be closer to the source of the issue. But if the required changes in Parse.File have wide implications, it may be easier to just do the change in the S3 adapter as you proposed.

[sarsonj]

@mtrezza I will ty to look on it on Parse side, I agree that it will be better to fix it there.

[mtrezza]

Converted to draft as the fix will likely be made in Parse Server.

[mtrezza]

Closing this as the fix has been made in Parse Server by parse-community/parse-server#7300.

@sarsonj If that PR does not fix this issue, please comment and we can reopen this PR.