Currently the module export binary files as raw binary content. Which makes the file impossible to edit.

Now I'm getting timeout, maybe due to an odd character that PHP is not capable of parsing.

Wouldn't it be better if we encode all binary data as base 64?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kasperg’s picture

Status: Active » Needs review
FileSize
1.66 KB

Agreed. Running coder review on modules with exported data results in all kinds of warnings.

Here's a patch for simple encoding/decoding data while maintaining backwards compatibility.

wjaspers’s picture

This is also extremely useful for those of us who modify features in-code rather than continually re-exporting them. Love the idea behind the patch, but, IDK the file field API well enough yet to say whether its correct. Just glancing at it, it looks like your patch only accounts for images ... this should be capable of handling any file.

pcho’s picture

Status: Needs review » Patch (to be ported)

At the current moment, the module only supports image formats (for the time being). Considering the binary data is being stored within the features file, it might not be optimal to openly support any other formats due to file size complexities (well, let's try featurizing a 50MB pdf). I am looking into the best way to handle this at the moment.

For now, I applied kasberg's patch as it solves the base64 encoding issue with images. It will be included as part of the next release. Thanks!

populist’s picture

Here is a reroll in the -p0 format. No other changes have been made and, yes, I know its in the new version.

frankcarey’s picture

So, why encode the file right into the features include anyway? You could store an md5() of the file to check if it's been overridden?

DamienMcKenna’s picture

Status: Patch (to be ported) » Needs work

A better status.

laken’s picture

In my use, the module is successfully encoding/exporting a filefield with a PDF attachment, and it is creating and linking the node to the file upon feature enable/import.

However for our use case (many nodes with many attachments, some large) it is unwieldy to have all these binaries encoded base-64 and stuffed into the features.content.inc file. That's going to make it many, many MB in size. Currently it's at only 7MB for a single test PDF and I can barely open the features.content.inc file in my code editor!

I am wondering if there's an existing option – or if it's possible to code an option – to specify that the files are going to be placed into the `/files` directory directly, and that `defaultcontent` should not encode them on export, or decode and create the files on import - it should merely set the field URI to point to.

I understand that a Feature should be self-contained apart from specified dependencies, and the current solution meets that requirement well, but for me it's inflexible. What do you think?

P.S. I'm also seeing the 'creating multiple files' bug on import that has already been reported.

populist’s picture

I have been using this patch in Panopoly for awhile and think it has a great experience for folks who want to export images as part of their default content. All they need to do is provide machine names to their content, use features to kick out one file, and then include it that file in their features.

I realize this creates *large* files, but since these files are conditionally included only when Features is installing them perhaps it is not a huge issue? I personally would favor the DX experience of this patch over something that is a bit more complex, but if we run into performance/memory issues it might be a non starter.

populist’s picture

As far as I can tell, this patch has been committed to both branches (http://drupalcode.org/project/defaultcontent.git/commit/1c61828 and http://drupalcode.org/project/defaultcontent.git/commit/c097a7b) and generally base 64 encodes files as part of the process.

Might it be better to close this issue and open new issues for additional issues around everything? Especially if a new release is rolled, its going to get confusing.

populist’s picture

Status: Needs work » Fixed

I am going to mark this as fixed as per #9

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.