IBX-368: Added image path normalization before storing it into the DB

[mateuszbieniek]

Question	Answer
JIRA issue	IBX-368
Bug	yes
Target version	7.5+
BC breaks	no
Tests pass	yes
Doc needed	yes

When Flysystem stores the image, the path is normalized and (for example) unprintable utf8 characters are dropped. The same transformation is not applied to the path stored in the ezimagefile table, which leads to 404 errors.

As the issue did not occur in legacy, an image path normalization command has to be implemented for migrating projects.

TODO:

• Implement feature / fix a bug.
• Fix new code according to Coding Standards ($ composer fix-cs).
• Implement an image path normalization command for projects migrating from legacy.
• Ask for Code Review.

[glye]

Very nice & compact. I'm only wondering if this is the right place for a binding to Flysystem, given that other handlers can be used? Other handlers might have other normalising rules, even stricter than Flysystem, so this might not be correct/enough.

Second, might we need this for non-image files too?

[mateuszbieniek]

Very nice & compact. I'm only wondering if this is the right place for a binding to Flysystem, given that other handlers can be used? Other handlers might have other normalising rules, even stricter than Flysystem, so this might not be correct/enough.

Second, might we need this for non-image files too?

1. I don't think we are expecting multiple "handlers" for a single field type - any idea for the other placement?
2. Binary files are somewhat not affected.

[glye]

I didn't mean multiple, just that Flysystem is documented as "default", implying it can be replaced by something else, so seems odd to have Flysystem code in the generic ImageStorage class.

Where else? Not sure, but I imagine the flysystem binarydatahandler could do it, and afterwards ImageStorage::storeFieldData could check with the returned binaryfile object if the filename is still the same as it was in the create struct. If not, update the $targetPath used further on in storeFieldData.

Or we could ask the ioservice up front if it wants to modify the name, and the ioservice asks the configured handler, which returns the corrected name, which we then use in the create struct. A bigger code change, but isn't it how we usually roll with regards to separation of concerns?

Anyway, if Andrew is happy, I'm happy :)

[mateuszbieniek]

I didn't mean multiple, just that Flysystem is documented as "default", implying it can be replaced by something else, so seems odd to have Flysystem code in the generic ImageStorage class.

Where else? Not sure, but I imagine the flysystem binarydatahandler could do it, and afterwards ImageStorage::storeFieldData could check with the returned binaryfile object if the filename is still the same as it was in the create struct. If not, update the $targetPath used further on in storeFieldData.

Or we could ask the ioservice up front if it wants to modify the name, and the ioservice asks the configured handler, which returns the corrected name, which we then use in the create struct. A bigger code change, but isn't it how we usually roll with regards to separation of concerns?

Anyway, if Andrew is happy, I'm happy :)

Oh, I understand now - I'll give it a second look. Thanks @glye .

[alongosz]

@ezsystems/documentation-team proofread the messages to users please (pointed out below)

[mateuszbieniek]

pinging for a second round of review:
@alongosz @adamwojs @glye

[glye]

LGTM, some questions aside.

[tomaszszopinski]

QA approved on eZPlatform-ee 2.5 & 2.5.20 with patch & diff.

[adamwojs]

Could you please merge up changed @mateuszbieniek?

[mateuszbieniek]

Merge up PR: ezsystems/ezplatform-kernel#197