Images should be stored inside the specific field info instance file directory

Hi itamair, I like this, thanks again for providing a patch. I'll test it on my local and commit it.

Added a new version of the patch with a few added comments and renamed a variable. Posted an interdiff. Wondering if there's a way to update existing files... Maybe this? https://www.drupal.org/project/fe_paths

I'm thinking this feature could be enabled via the POST payload, ie. "filename_after_entity_field: True", to avoid ending up with a mixed naming scheme inadvertently.

There are modules that could provide a post-renaming of all the files (for a clen files space). However, I've never used any of them, and cannot find any proper documentation on how you would do that.

I will post a patch soon.

@mariano.barcia - Curious if you've made any progress on this since last year. We are testing out this module for use with the farmOS distribution (http://drupal.org/project/farm). I want to make sure that files are saved to their proper subdirectories, and not all dumped into the public:// directory.

Would you say that the patch in #5 is good enough to start with? Updating of existing files is less of a concern to me, because most files are archived in farmOS, and rarely get changed.

Hello m.stenta,

I think patch #5 works fine if you start from scratch.

I recall I stopped working on this because, as a module maintainer, I needed to avoid disruption with previous versions when upgrading from previous versions.

The cheapest solution would be to add a parameter in the POST request to enable this functionality, so that all new clients can start using it, while keeping the old clients safe if they upgrade.

I could commit a solution like that right away if you provide a patch I can review, thank you.

Finally got around to testing this patch - and it works!

However, I ran into a related issue: if the file directory does not already exist, then the upload does not work. The entity is created, but it doesn't have any file attached to it. Presumably it is uploading the file to /tmp, but the attempt to move it to the correct directory fails, so it ends up in limbo.

I was able to automatically create the necessary directory using file_prepare_directory(). However it still isn't moving the file there from the temporary directory. Continuing to debug... will post a patch if/when I figure it out.

I got it working (minor mistake in my code). Attached is a patch that uses file_prepare_directory() to create the directory first, as well as an interdiff.

Hi m.stenta, thanks for working on a patch for this.

However, I'm not yet convinced this patch is the best way to deal with this feature. Basically, I don't think restws_file needs to have a say on or get in the way of how Drupal is storing the files locally.

Take a look at ie. this issue in drupal core. This is an upcoming change in D7 that will change the default files/images location:
https://www.drupal.org/project/drupal/issues/2128055

Also, there is a module https://www.drupal.org/project/filefield_paths that could solve this in a better way.

I'd rather integrate restws_file with drupal core and filefield_paths.

Integration with drupal core would mitigate the basic risk of thousands of files in a single dir, and will store files in paths like field/[field_name]/[date:custom:Y]/[date:custom:m] by default (see the patch https://www.drupal.org/files/issues/2128055_44.patch). That alone is more than what this patch provides.

Integration with filefield_paths would allow for:
a) much more flexibility in naming
b) retroactive updates (which has been my concern above)

I'm keen on hearing your thoughts on this m.stenta. Thanks again.

I'm not yet convinced this patch is the best way to deal with this feature.

Perhaps not, but it is an improvement.

I don't think restws_file needs to have a say on or get in the way of how Drupal is storing the files locally.

I agree, in general. However, right now restws_file is already overriding Drupal's default behavior.

File/image fields have a setting for specifying the directory that the files get saved to. In the patch it is the variable $value['settings']['file_directory']. This module is not respecting that setting currently - it is just saving all files to the root files directory (public://).

Also, there is a module https://www.drupal.org/project/filefield_paths that could solve this in a better way.

Filefield Paths may provide some nice features, but is not a core module, and should not be a hard dependency of restws_file. It provides more flexibility to override the $value['settings']['file_directory'] setting. It would be fine if restws_file can also use that override (only if filefield_paths is enabled), but it should not depend on that.

As an aside, I made the decision to completely remove Filefield Paths from my distribution and revert to using the core file_directory setting because I couldn't justify the added complexity of Filefield Paths. I don't think we should expect all sites to need or want Filefield Paths. It is very useful - but not always necessary - and minimizing dependencies is a good thing.

In my mind, this patch is a good first step, because it at least makes restws_file respect the $value['settings']['file_directory'] setting (thus bringing it more inline with the patterns of core).

If the patterns of core change, then that should be a follow-up issue to this one - if and when that patch lands in D7. I haven't reviewed that patch in depth, but it sounds like it is simply changing the default directory - but will still allow overrides. So we still need to respect the $value['settings']['file_directory'] setting regardless in this module.

Regarding filefield_paths - that can also be a separate feature request issue, I think. No need to complicate this one. That would be an enhancement, whereas this one is more of a bug report in my opinion. And perhaps the issue of "migrating old files to new paths" could be a separate issue as well (and perhaps Filefield Paths is a solution).

Those are my thoughts. I didn't set the issue status to "Needs Review" because I wasn't sure if we wanted to do more in this patch. But for my purposes it works great as-is.

PS: Thanks again for this module - it really saved me a lot of time! :-)

Right, thanks for clarifying.

So this patch effectively IS the integration with drupal core I was mentioning. Great!

I would happily commit this patch once I can test it, and release it either as 1.3 or as 2.0.

What do you reckon will happen, if a user upgrades from the current 1.2? Is there a way to document (at least warn, or give advice) on what would happen to the file paths from 1.3 (or 2.0) on?

Glad to hear this module is useful :-) Cheers
--m

So this patch effectively IS the integration with drupal core I was mentioning. Great!

Yes! I think so! :-)

What do you reckon will happen, if a user upgrades from the current 1.2?

My understanding is that it is a "safe" upgrade path.

The file/image fields store the path to each uploaded file - so even if the directory setting is changed, it still remembers where previously-uploaded files are stored - and won't touch those. New files will be uploaded to the new directory, but old ones won't be moved. So it does mean that existing sites will have some files stored in the root public:// files directory, and some in the new directory - but it won't cause any issues, as far as I know.

It is a similar situation if you were to change the file_directory setting itself (regardless of using restws_file) to a different directory. The core file module(s) do not move old files to the new directory - they leave that up to you if you want to reorganize.

Filefield Paths provides options for reorganizing old files, so I think we can defer to that module and recommend it for people who want to clean up old files. It doesn't need to be the responsibility of this module, in my opinion.

So perhaps just explaining that a bit in the release notes of the next version would be enough.

Right, thanks for the feedback.

When you go and change the paths, even though Drupal core will not move the files for you, it's you the one manually changing that.

But in this case, it would be a side-effect of updating this module.

I guess I can live with that, pointing it out in the project page, given that this patch plays nicely with Drupal core.

I'd add something like:
- From version 1.3 on, files are saved according to the filefield path configured at <...>
- If you want to re-organize your files, previous to or after updating from v1.2, I recommend you use the filefield_paths module

Hopefully I can get 1.3 out during the week-end. Cheers.

quick update: Tested and verified this patch works.
filefield_paths also working as expected.

Releasing 1.3 now

🎉 Thanks @mariano.barcia! :-D