Bug #24884

pagetree->expand branch destroys parts of BE_USER->uc

Added by Rupert Germann over 8 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
Start date:
2011-01-28
Due date:
% Done:

100%

TYPO3 Version:
4.6
PHP Version:
5.2
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

Clicking "expand branch" twice in a huge pagetree expands all pages (~thousand in my case). Besides the fact that this feature does not make much sense in a huge pagetree it resets parts of the saved settings in BE_USER->uc which is quite anoying.

make some changes in the "user settings" module that differ from the default values, or click some of the checkboxes below the list module (e.g. enable the localisation view)

Then click "expand branch" twice in a huge pagetree. When you think the pagetree is fully expanded reload the list module or check the values in user settings.

All changed settings are set back to their default values.

(issue imported from #M17396)

uc_before.png View (78.3 KB) Administrator Admin, 2011-01-28 22:17

uc-after.png View (19.2 KB) Administrator Admin, 2011-01-28 22:18

24884.diff View (5.54 KB) Jo Hasenau, 2011-07-21 14:48


Related issues

Related to TYPO3 Core - Bug #24882: pagetree->expand branch does not work as expected Closed 2011-01-28
Related to TYPO3 Core - Bug #36238: Pagetree lost for non admin after upgrading to 4.5.15 Closed 2012-04-17

Associated revisions

Revision b710bae6 (diff)
Added by Jo Hasenau about 8 years ago

[BUGFIX] Saving Page Tree states issues in large environments

Change the way expanded nodes are saved for be_users back to a simple
key/boolean pair and adjust the size of the uc field to fit the needs of
larger page trees.

Change-Id: Ic926080b0dc0e6e24226ab00554091a5f0ffae66
Resolves: #24884
Releases: 4.6
Reviewed-on: http://review.typo3.org/3912
Reviewed-by: Jo Hasenau
Tested-by: Jo Hasenau
Reviewed-by: Philipp Gampe
Tested-by: Philipp Gampe
Tested-by: Christian Kuhn
Reviewed-by: Steffen Gebert
Tested-by: Steffen Gebert

Revision d92797cf (diff)
Added by Jo Hasenau over 7 years ago

[BUGFIX] Saving Page Tree states issues in large environments

Change the way expanded nodes are saved for be_users back to a simple
key/boolean pair and adjust the size of the uc field to fit the needs of
larger page trees.

Change-Id: Ic926080b0dc0e6e24226ab00554091a5f0ffae66
Resolves: #24884
Releases: 4.5
Reviewed-on: http://review.typo3.org/10348
Reviewed-by: Markus Klein
Tested-by: Markus Klein
Reviewed-by: Stefan Galinski
Tested-by: Stefan Galinski

History

#1 Updated by Stefan Galinski over 8 years ago

It's not the pagetree that touches your user configuration, but the state provider. Which parts of your configuration are resetted? Just the tree state?

#2 Updated by Rupert Germann over 8 years ago

the section "moduleData" is emptied (probably because it has no default)
the section "backendComponents" is completely missing
other values - e.g. "titleLen" are resetted to their defaults

see screens for details

#3 Updated by Rupert Germann over 8 years ago

while analyzing some queries in BE I found a possible reason for the broken/resetted uc:
with a fully expanded tree (1000pages + ) the Pagetree tries to write roughly 90KB Data in a mysql TEXT field (allowing only 64KB)

But please don't "fix" this by just enlarging the field to mediumtext.
the question is, why is the pagetree writing a serialized array with 89348 entries for ~1000 visible pages?!

#4 Updated by Stefan Galinski over 8 years ago

Just an assumption: The rootline of the expanded nodes is written into the user configuration. Seems that we need to finetune the whole state providing mechanism, but it should be possible. Thanks for the investigation, Rupert!

#5 Updated by Jo Hasenau about 8 years ago

  • Target version changed from 0 to 4.5.4

Since we had similar problems with a project having > 10.000 pages we started investigating on this issue.

The current version of the state provider creates a key/value pair with the ID of a node as the key and the full path to this node as the value.

mp-0-12345 = /mp-0-0/mp-0-1/mp-0-12/mp-0-123/mp-0-1234/mp-0-12345

When the user does a relogin this information will be used to create the last visible representation of "his" tree via ExtJS' expandPath function.

Of course this generates a huge overhead on the DB-side since a string like that has to be kept for each and every visible node, on the other hand it generates the same overhead on the client side, since these strings are kept in memory, while ExtJS handles the tree for the user.

The first thing we changed was the ID itself, since it always contains the "mp-0-" part even if there are no DB mounts at all. We changed it to "p12345" for pages without any mounts and "p12345-123" for pages belonging to DB-mount 123, so we can still be sure to have unique IDs but with less overhead.

After that we reduced the stuff to be kept in memory and to be saved to BE_USER->uc by cutting off the path completely. Instead the key/value pair now looks like this

p12345-123 = 1

and for pages without any mount it's even shorter

p12345 = 1

This will reduce the overhead significantly especially for deeply nested page trees.

We were able to do so because we found out that it's just not necessary to save the full path for each node. Instead it is much easier to expand nodes recursively while opening the tree, since you just have to call the restoreState function on each expand.

This way you make sure that the tree checks for new IDs that have been loaded during the last expand action and expands these as well, if they can be found in the array of visible nodes.

We still have to change the field type from TEXT to MEDIUMTEXT or maybe even LONGTEXT to make sure it can take even a few thousand entries with larger UIDs, but we were able to make a really large tree usable this way.

Currently we are investigating if there might be any side effects, but it looks promising. So hopefully I will be able to provide a patch until tomorrow afternoon.

IMHO this is a bug that must be fixed even in 4.5.x since the current version makes the system unusable for users with large pagetrees especially when these trees have got a deeply nested structure, which is why I set the target version accordingly. Someone should change it to "must have" as well.

#6 Updated by Jo Hasenau about 8 years ago

It could even be useful to convert the uids used to create the ID of the page tree entries from DEC to HEX to reduce the length of the generated strings up to 50%.
And we could even get rid of the "p" since this doesn't have to be saved to the DB at all, because it is just necessary for W3C compliance of the HTML-Attribute and therefor can be added while rebuilding the tree.

#7 Updated by Stefan Galinski about 8 years ago

Wow, that sounds like a real performance booster patch as this is also a show-stopper while using mobile devices. Performance improvements are definitly allowed to go into the 4.5 branch, so don't hesitate to add this to the "Release" tag of your review request.

#8 Updated by Jo Hasenau about 8 years ago

Still not able to push anything from this workstation, so here we go with an "old school" DIFF :-)

#9 Updated by Mr. Hudson about 8 years ago

Patch set 1 of change Ic926080b0dc0e6e24226ab00554091a5f0ffae66 has been pushed to the review server.
It is available at http://review.typo3.org/3912

#10 Updated by Mr. Hudson about 8 years ago

Patch set 2 of change Ic926080b0dc0e6e24226ab00554091a5f0ffae66 has been pushed to the review server.
It is available at http://review.typo3.org/3912

#11 Updated by Jo Hasenau about 8 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

#12 Updated by Steffen Kamper about 8 years ago

  • Status changed from Resolved to Needs Feedback
  • Target version deleted (4.5.4)
  • TYPO3 Version changed from 4.5 to 4.6

Please push a patch for 4.5 as well! Thanks.

#13 Updated by Steffen Kamper about 8 years ago

  • Target version set to 4.5.5

#14 Updated by Oliver Hader about 8 years ago

  • Target version changed from 4.5.5 to 4.5.6

#15 Updated by Chris topher almost 8 years ago

  • Target version changed from 4.5.6 to 4.5.8

#16 Updated by Ernesto Baschny over 7 years ago

  • Target version changed from 4.5.8 to 4.5.12

#17 Updated by Gerrit Code Review over 7 years ago

  • Status changed from Needs Feedback to Under Review

Patch set 1 for branch TYPO3_4-5 has been pushed to the review server.
It is available at http://review.typo3.org/10348

#18 Updated by Jo Hasenau over 7 years ago

  • Status changed from Under Review to Resolved

#19 Updated by Thomas Deinhamer over 7 years ago

So, does this mean a DB update is needed when updating to the next TYPO3 4.5 bugfix release for all installations?

#20 Updated by Stefan Galinski over 7 years ago

You don't need to change the field, but I would recommend it, because it can prevent possible bugs caused by huge user configurations/session data like the rootline informations of the pagetree. Your decision...

#21 Updated by Riccardo De Contardi almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF