views_handler_field_field::set_items() does this:

    $display = array(
      'type' => $this->options['type'],
      'settings' => $this->options['settings'],
      'label' => 'hidden',
      // Pass the View object in the display so that fields can act on it.
      'views_view' => $this->view,
      'views_field' => $this,
      'views_row_id' => $row_id,
    );

If the field formatter outputs a form (commerce_add_to_cart_form, for instance), $display gets stored in form_state.
This means that each row stores the entire view in form state, leading to entries in the db that are 8-9mb each in certain cases (related issue: #2057073: Cache form table saves very large records when forms are displayed in a view).

Those three params were added for the editablefields module, I remember that.
We can't remove them without breaking the module (which is quite broken at the moment anyway) and any others.
However, what we can do is introduce a minor BC break, replacing views_view with views_view_name.
This allows any module to load the view when needed, while keeping it out of form state, eliminating most of the baggage.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz’s picture

Status: Active » Needs review
FileSize
969 bytes

Easy.

bojanz’s picture

Of course, I could add a workaround to Commerce, unsetting parts of $context that get put in form state, but this seems like a sensible change anyway.

bojanz’s picture

Status: Needs review » Closed (won't fix)

Added a workaround in #2057073: Cache form table saves very large records when forms are displayed in a view instead, since views_field needs to be unset as well, which is not an option because of the BC break. Also, the situation is pretty much limited to Commerce.

xjm’s picture

Issue summary: View changes
Status: Closed (won't fix) » Active

Since this is pretty nasty when it manifests itself on any exposed filter with an unreasonably large list, we could probably do something to be a little more defensive about what we stuff in the cache table. The correct solution is to use a sensible form element for the situation, but it's totally conceivable for a field's values to evolve a way the view builder doesn't anticipate and make the site melt before someone tracks down the problem. So reopening for some more discussion.

I also think I've run into problems in the Views UI simply trying to configure a field that has lots of potential values (when working on d.o features for example).

Darth_Beholder’s picture

I have a view with editable fields (from editablefields module).

Every time i refresh my view page, cache_form grows by 1,5-2mb.
After applying this patch, it grows rapidly, by 9-10mb at each refresh.

I check this twice. Any suggestions?

maximn’s picture

there is still a huge problem with cache_form and views. i have ubercart installed and add to cart forms inside a view and my cache_form table grows like crazy - 1.5-2 gigabytes in couple hours, and this is just a test site with no real traffic.

i applyed the path with no change in behaviour at all, so i decided to make some further investigation on this. turn out to be, that the whole 'view' objet is cached on each form caching, that's why cache table grows so fast. in my case the problem was in file views_plugin_row_node_view.inc::render()

  function render($row) {
    if (isset($this->nodes[$row->{$this->field_alias}])) {
      $node = $this->nodes[$row->{$this->field_alias}];
      $node->view = $this->view;
      $build = node_view($node, $this->options['view_mode']);

      return drupal_render($build);
    }
  }

as you can see, whole view object is put into $node and later $node is cached by form_cache mechanism. i'm not sure why is that made like that, so i just made small hook in hook_form_alter to unset this property before it is cached.

function hook_form_alter(&$form, &$form_alter, $form_id) {
  if (isset($form['node'])) {
    $node = $form['node'];
    if (is_object($node['#value']) && property_exists($node['#value'], 'view')) {
      unset($node['#value']->view);
    }
  }

  foreach ($form_alter['build_info']['args'] as $arg) {
    if (is_object($arg) && property_exists($arg, 'view')) {
      unset($arg->view);
    }
  }
}

i'm sure there is some better workaround, but this worked fine for me

rvillan’s picture

Here is the patch updated for 3.x-dev.

joelpittet’s picture

Status: Active » Needs review
dawehner’s picture

Status: Needs review » Needs work

It would be great if we could not break BC by default but instead have some checkbox (just on 7.x-3.x but not on D8) which removes the view from it.

joelpittet’s picture

Just throwing this out there, could we just not cache the form?

I had a bit of success with just doing and ['#after_build'][] = 'MODULE_after_build' callback for views_form_commerce_cart_default

By setting this in the callback:

function MODULE_after_build($form, &$form_state) {
  $form_state['no_cache'] = TRUE;

  return $form;
}

My huge cache_form serialize/unserialize just went away, *POOF*. And things still *seemed* to continue to work... but I guess this would need more tests but let me know if you know of a scenario which this would be a bad idea?

FYI, i'm of the opinion form and form_state are not caches in the first place but if we can remove them from views successfully that may be a big win... Here is an experiment inline with that to those who are interested #2183275: Use cache for $form, kv for $form_state


I'm using the patch in #7 successfully but still have large form and form_state because views_handler_field_field::post_execute() loads the entire entity on the results special field called _field_data and the entire View still gets added via the $form_state['build_info']['args'][0]

joelpittet’s picture

A better docs reference #689084-12: Document $form_state['cache'] for #no_cache. It seems to have some uses with #ajax forms and multistep forms. So in my cart example above it would only be fine if I didn't turn ajax on that view I'd assume, which I'd like to do... later;)

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
4.15 KB
tim.plunkett’s picture

+++ b/plugins/views_plugin_exposed_form.inc
@@ -157,7 +157,6 @@ function render_exposed_form($block = FALSE) {
-    $form_state['exposed_form_plugin'] = $this;

This is the key line from the D8 patch.

tim.plunkett’s picture

Issue tags: +Needs tests

Status: Needs review » Needs work

The last submitted patch, 12: 2059555-views-12.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
4.22 KB
976 bytes

D8 has solved a lot of problems through ViewExecutable and PluginCollection

Status: Needs review » Needs work

The last submitted patch, 16: 2059555-views-16.patch, failed testing.

shawn_smiley’s picture

Note that with the patch in #16 that we see a large number of unserialization errors: "Erroneous data format for unserializing 'view' cache.inc:439".

I haven't had a chance to track down the source of the issue though.

gumrol’s picture

For anyone using Ubercart and experiencing the same issue - the proposed solution in #10 by @joelpittet works like a charm, just check the caveats he mentions in #11.

The below is a workaround, and won't work in all cases. In my case, I have no "add to cart" forms on my product listing pages. My cache_form table grows in Gigs per day caching these forms needlessly, so a drastic measure is needed:

Use a hook_form_alter (dynamic in Ubercart, so you need to make provision for this) to register your '#after_build' callback, only when the key 'view' is available on the node entity isset($form['node']['#value']->view). (You can see why this is not an ideal solution). Your callback function will contain exactly what @joelpittet puts forward in #10 - thanks Joel!

cache_form on my site now stays trimmed with the cron and is in the Mb size range instead of Gb.

michfuer’s picture

Status: Needs work » Needs review
FileSize
4.23 KB
1.59 KB

Using the patch in #16, I'm seeing some double digit reductions (~20%) in the size of form_state_form-* records `data` column.

Running into some errors, so re-rolled.

For the "Erroneous data format for unserializing 'view' cache.inc:449" errors I had to manually truncate the `cache_views` table. Relevant info from https://github.com/doctrine/doctrine2/issues/3897

What happens now when a class that implements \Serializable is that a "Warning: Erroneous data format for unserializing" shows up and the function unserialize() returns false.

That is because a class that implements \Serializable is expected to have the letter 'C' in the serialize string instead of the letter 'O'.

Status: Needs review » Needs work

The last submitted patch, 20: 2059555-views-20.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

DamienMcKenna’s picture

I can confirm the test failures locally. I'll try to look at them soon.

michfuer’s picture

I'm not having any luck reproducing the test failures locally. I've got the latest 7.x dev version of Drupal core, and 7.x-3.x-dev of Views with the patch in #20 applied. I see a few exceptions (most related to this issue https://www.drupal.org/node/2331209), but no failures.

$ php scripts/run-tests.sh --verbose "Views Plugins"

Drupal test run
---------------

Tests to be run:
 - Access (ViewsAccessTest)
 - Argument_default (ViewsArgumentDefaultTest)
 - Argument validator (ViewsArgumentValidatorTest)
 - Cache (ViewsCacheTest)
 - Exposed forms (ViewsExposedFormTest)
 - Pager (ViewsPagerTest)
 - Display plugin (ViewsPluginDisplayTestCase)
 - Jump menu (viewsPluginStyleJumpMenuTest)
 - Style: Mapping (ViewsPluginStyleMappingTest)
 - Styles (ViewsPluginStyleTestCase)
 - Style: unformatted (ViewsPluginStyleUnformattedTestCase)
 - Tests user argument default plugin (ViewsUserArgumentDefault)
 - Tests user argument validator (ViewsUserArgumentValidate)

Test run started:
 Wednesday, October 4, 2017 - 10:50

Test summary
------------

Access 68 passes, 0 fails, and 5 exceptions
Argument_default 19 passes, 0 fails, 7 exceptions, and 4 debug messages
Argument validator 6 passes, 0 fails, and 2 exceptions
Cache 25 passes, 0 fails, 5 exceptions, and 12 debug messages
Exposed forms 102 passes, 0 fails, 10 exceptions, and 29 debug messages
Pager 92 passes, 0 fails, 20 exceptions, and 33 debug messages
Display plugin 6 passes, 0 fails, and 2 exceptions
Jump menu 6 passes, 0 fails, and 1 exception
Style: Mapping 43 passes, 0 fails, and 1 exception
Styles 19 passes, 0 fails, 3 exceptions, and 3 debug messages
Style: unformatted 18 passes, 0 fails, and 1 exception
Tests user argument default plugin 8 passes, 0 fails, 1 exception, and 2 debug messages
Tests user argument validator 16 passes, 0 fails, and 3 exceptions

Test run duration: 8 min 49 sec

Any advice on reproducing the test bot failures locally is much appreciated!

DamienMcKenna’s picture

@michfuer: I reran the tests for the patch and don't see any exceptions without the patch, but all of those exceptions show when the patch is enabled. The exceptions need to be fixed, we won't commit the change as-is.