Reworking fe_block_settings export format.

I really love this idea. It would be a dramatic improvement over the current export format and I don't see any drawbacks. I'd love to get this in. I have two initial remarks:

• The role visibility patch should really be separated and committed in #868828: Capture role visibility settings in fe_block?, after this goes in.
• The array keys in the export should be sorted alphabetically. Features has a function for this: _features_sanitize()

Also the tests should pass and we should provide some additional tests, for example to test the conversion of the old export format to the new.

A lot of code has moved in this patch (understandably) which makes the patch quite hard to read atm. If you can manage to separate out the role visibility and get the tests to pass I'll review this in more detail.

Great! Let's see what the bot says.

The Features-module itself uses it so I guess we should follow suit. We do lose some code legibility, but on the other hand we win consistency, and patch readability.

OK I guess you're right, if Features runs the data through _features_sanitize() when it is doing the comparisons we don't really need it here. ksort() is not sufficient btw, it does not recurse through the arrays when sorting.

Thank you guys for all the work being done here. We are relying heavily on Features Extra for initial block configurations in our university deployment. Will the work being done here require re-exporting of blocks and block configuration once it is committed?

@vinmassaro, no the goal is to keep it transparent. It will support both the old and new export formats. When you update your features it will be converted to the new format automatically.

We were running pfrenssen's sandbox code before it was moved into Features Extra. We're now running the latest dev version.

@derhasi any idea if this restructuring fixes the bug I am seeing in #1830180?

@derhasi I'm going to try to test your patch out later today so we can get some action in here.

I hope to be able to review this soon, am a little tied up with work atm.

Here are my findings after applying patch from #13 to 7.x-1.x. I am running Features 7.x-2.0-beta1. I patched and re-exported my feature since it showed as overridden.

1. When editing any of the blocks with exported block settings, the visibility section is not editable:

2. I am exporting region settings for four themes. Only one theme is being set after enabling the feature:

Here's is what is happening in code. The value for key 'theme' appears that it should match for each theme but yale_standard is being set for each one:

  $export['sharethis-sharethis_block'] = array(
    'cache' => '1',
    'custom' => '0',
    'delta' => 'sharethis_block',
    'module' => 'sharethis',
    'node_types' => array(
      0 => 'event',
      1 => 'news',
    ),
    'pages' => 'news/*
videos/*
slideshow/*',
    'themes' => array(
      'seven' => array(
        'theme' => 'yale_standard',
        'status' => '1',
        'weight' => '-16',
        'region' => 'content',
      ),
      'yale_boxed' => array(
        'theme' => 'yale_standard',
        'status' => '1',
        'weight' => '-16',
        'region' => 'content',
      ),
      'yale_standard' => array(
        'theme' => 'yale_standard',
        'status' => '1',
        'weight' => '-16',
        'region' => 'content',
      ),
      'yale_wide' => array(
        'theme' => 'yale_standard',
        'status' => '1',
        'weight' => '-16',
        'region' => 'content',
      ),
    ),
    'title' => '',
    'visibility' => '1',
  );

I have reviewed the exporting part. but still have to do the importing and the converter for the old export format. It looks great! You did major refactoring and cleanup of the code too, awesome work!

I only looked at the code for now, still have to actually run it. I also still want to check the bug reported by vinmassaro in #24.

Updated patch contains documentation cleanups, some small optimizations and I renamed two functions.

@pfrenssen: Please ignore the first part of #24. I just realized it is from a misbehaving Chrome extension.

I just tested your patch in #25 and when exporting my feature under the new version, it tries to change all of my region visibility settings to the same theme (as seen in #24). The page visibility bug I am seeing in http://drupal.org/node/1830180 appears to be fixed now.

I am having trouble tracing back the issue in #24. I *think* it is caused by _fe_block_info_by_theme() in the patch from #25. $theme in this function only has one value and it is set to your currently enabled theme. This is then set as the value for each 'theme' key.

New version of the patch:

• Fixes problem from #24
• Bumped export format to 2.0, it is a significant change after all :)
• Replaced the calls to the evil _block_rehash() function with a faster version that does not write all blocks to the database without reason. This sped up the export of my test feature from 5.7 seconds to 0.8 seconds.
• Sorted the theme specific settings.
• Documentation improvements.
• Renamed some variables and a function.
• Small optimizations.

Still have to look at importing and the backwards compatibility layer.

Did some performance benchmarking by running drush fl on a site with 35 enabled features and 3 themes:

Before (with _block_rehash()):

1. 2m3.760s
2. 1m57.917s

After (with _fe_block_get_blocks()):

1. 0m6.873s
2. 0m6.937s

This is amazingly 17.5x faster!

Awesome! As I mentioned earlier, we are really dependent on Features Extra in our university D7 install, so your work on this issue is much appreciated. I will be testing this updated patch today or tomorrow and will let you know how it works out.

Aww it was too good to be true. This patch removes the drupal_static() from _fe_block_get_blocks(). A bit slower now but at least the tests pass.

@vinmassaro: excellent! I'm looking forward to it!

New benchmarks of time drush fl:

without patch:

1. 1m51.628s
2. 2m9.683s

with patch from #32:

1. 0m8.060s
2. 0m7.917s

Still 15x faster than before, can't say I'm very disappointed :D

This patch is working great. I applied the patch and re-exported my feature, and the code changes looked correct. I then installed the feature and the block configurations and visibility settings are correct. The block settings file is way cleaner. Very nice work!

Awesome news! This has been committed to 7.x-1.x-dev: 8cc38ed. Thanks!!