Flawed metadata support

[stof]

In the Amazon adapter, metadata are used from memory when renaming a file, but the metadata are never populated from the persistent ones. The same issue exists for getMetadata: it will return them only in the process defining them as it won't read them from amazon

[TomiS]

I'm noticing something similar with GridFS adapter.... Actually, I'm at the moment browsing through the code and it's seriously difficult for me to understand how could I create/write a file with metadata with current api. There are setters and stuff in many classes but e.g. in Filesystem class there is no way to set metadata. I recall it was previously a part of write method.

This stuff is urgent for me at the moment and I can even contribute code. I'd just like to know what's the plan with this... It seems the api has changed quite a lot since my last visit here (which probably is mostly good), but anyway...

@l3l0 Can you tell me anything?

[TomiS]

One option would be to refactor Filesystem class to support following

$file = new File();
$file->setKey($key);
$file->addMetadata('key1', 'value1');
$file->addMetadata('key2', 'value2');
// or
$file->setMetadata($arrayOfKeysAndValues);
$file->setContent($bytes);
$filesystem->writeFile($file); //throws exceptions if key or content are not set

Alternatively, the metadata could be set in write function as an optional third parameter (as it was before, I think...)

$filesystem->write($key, $content, $metadataArray);

Finally, I think File object should be as fully populated as possible when one fetches it using $filesystem->get($key). In most cases it means the actual storage must be queried immediately when calling get but the loss in performance is compensated in clear and consistent behavior of the library.

[l3l0]

Hello,

I think the main problem is in all implementations of Gaufrette\Adapter\MetadataSupporter interface (like grid or s3 adapter), cause as @stof noticed right now metadata is not populated from existing files. IMO getMetadata should read metadata for key and setMetadata should rewrite it for key (without write file content if possible)... maybe we should add addMetadata or something similar to MetadataSupporter I am really not heavy amazon/gridfs user (amazon API expert neither) so I am waiting at suggestions :)

About api I dont see the problem to fix it somehow (but I do not want to do BC breaks really). We can add setMetadata and getMetadata to filesystem. Right now we can access metadata setter and getter from filesystem only by:

if ($filesystem->getAdapter() instanceof Gaufrette\Adapter\MetadataSupporter) {
   $filesystem->getAdapter()->setMetadata($key, $metadata);
}

[stof]

@l3l0 I don't really think we need a metadata accessor on the Filesystem. They are related to files, not to the filesystem, so the API for them should be in the File object.

I like the first proposal of @TomiS above

[l3l0]

:+1: then :) /cc @TomiS do you want to do that ?

[TomiS]

@l3l0 Yep, I start working on it now.

[stof]

The current implement still suffer of the issues noted above: for instance, getMetadata is totally wrong in the S3 adapter (it does not read the metadata) and setMetadata has to be called before writing the content, otherwise it is never saved. This also means that rename is flawed as it fails at copying the metadata.

Any news about a proper fix ? My app currently keeps using a forked version of the old API as it is the only way to be able to set the content-type on the S3 required to upload images properly. but this forbids me using any library relying on 1.4.

[TomiS]

@stof I think there's a slight chicken/egg problem. In my opinion Gaufrette would substantially benefit from the architectural changes I made in feature/metadata-refactoring branch. Those changes would help writing the adapters better and more stabile. On the other hand I recognize I might not be able to finish the work by myself because a) there are many adapters I'm not familiar to and I'm not able to refactor nearly all of them and b) I seem to have problems with spec tests because I don't know very well how to write them (specifically in tricky environments like this). One solution would be to define the new architecture as development branch for Gaufrette 2.0. That would make the architectural changes official and development for adapters could follow that. But I understood @l3l0 wouldn't want to create a new main version of the lib (which I also understand). Anyway, it's a bit tricky situation.

[wysow]

Closing this as it seems outdated now...

[stof]

@wysow this is not outdated. The metadata handling has not been changed since 2012 and is still flawed.

[akerouanton]

Wow.. I wasn't aware of this issue until @zyphlar created #451. I will try to deep dive into @TomiS's branch and see what needs extrawork.

EDIT: Unfortunately I don't think feature/file-metadata-refactoring branch will be easy to integrate because it's mostly a WIP with alot not specifically related to the issue described here. There's also lack of tests. Anyway, I will add this issue to the todolist in #440.

[TomiS]

@NiR- Yeah, unfortunately my refactoring attempts are pretty much dead code after all these years :)

[jspizziri]

@NiR- , did https://github.com/KnpLabs/Gaufrette/pull/451 resolve & close this issue?

[grempa]

Are there any new updates to this issue?