EZP-29451: As an Administrator I want to have a CLI command for database cleanup #2431

kmadejski · 2018-08-29T13:02:51Z

Question	Answer
JIRA issue	EZP-29451
Bug/Improvement	no
New feature	yes
Target version	`master`
BC breaks	no
Doc needed	yes

This PR introduces the new command which allows to clean up the database from the unwanted, archived content versions as well as unwanted drafts.

Big thanks for XROW (https://github.com/xrowgmbh) who provided POC for this command!

Open questions to reviewers:

Since this command might be considered as a reflection of legacy's flatten script (https://github.com/ezsystems/ezpublish-legacy/blob/master/bin/php/flatten.php), the question is whether we want to make it smart enough to clean up also temporary content classes and temporary roles as the previous script could do? I think that if we want these functionalities, then it should be done as a separated command.
Should iteration count be implemented there? In other commands, it's supposed to decrease memory usage, but this command operates on a simple array containing content IDs and version counter only. Each ContentInfo object is loaded within the loop, so it's always only one at the time.

TODO:

Implement feature.
Fix new code according to Coding Standards ($ composer fix-cs).
Ask for Code Review.

eZ/Bundle/EzPublishCoreBundle/Command/RemoveVersionsCommand.php

adamwojs · 2018-08-29T19:50:55Z

Should iteration count be implemented there? In other commands, it's supposed to decrease memory usage, but this command operates on a simple array containing content IDs and version counter only. Each ContentInfo object is loaded within the loop, so it's always only one at the time.

I think so, but the best way to find the answer will be to run command on some large database. Can you help @alongosz / @vidarl with this ?

kmadejski · 2018-09-04T09:42:42Z

@adamwojs thanks, requested changes are done in 7337b0d

kmadejski · 2018-09-05T09:43:32Z

Travis issue is already fixed. Could you please review @vidarl / @andrerom?

vidarl

Looks ok to me, but maybe invalid values for --keep option should throw InvalidArgumentException too?

kmadejski · 2018-09-13T13:57:39Z

@vidarl thanks, added $keep check in 5a90c66.

@adamwojs / @andrerom / @alongosz do you see anything else to improve here?

eZ/Bundle/EzPublishCoreBundle/Command/RemoveVersionsCommand.php

kmadejski · 2018-09-14T09:45:38Z

Thanks @alongosz. All requested changes are done in f843198.

eZ/Bundle/EzPublishCoreBundle/Command/RemoveVersionsCommand.php

…cleanup

…tabase cleanup

… for database cleanup

konradoboza

The code looks good to me, basic tests went ok. 👍

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php

kmadejski · 2018-12-17T15:34:33Z

@micszo fixed what you've found. Could you please do a second round of tests?

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php

kmadejski · 2018-12-19T14:47:50Z

@m-tyrala covered a case with keep = 0, please check once again.

m-tyrala

QA approve 🚀

lserwatka · 2018-12-19T19:03:08Z

@DominikaK we need a few bits of doc here

gggeek · 2019-05-17T08:24:48Z

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php

+
+        $this
+            ->setName('ezplatform:content:cleanup-versions')
+            ->setDescription('Remove unwanted content versions. It keeps published version untouched. By default, it keeps also the last archived/draft version.')


Nipick: 'unwanted' is not the best term to use here: it is not descriptive of what it is considered unwanted

gggeek · 2019-05-17T08:27:50Z

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php

+                'keep',
+                'k',
+                InputOption::VALUE_OPTIONAL,
+                "Sets number of the most recent versions (both drafts and archived) which won't be removed.",


Not sure having 1 value for the sum of both archived versions and drafts makes a lot of sense here.
I generally operate on the basis of "keep the last 4 archived versions", "keep all drafts", "keep no drafts".
I see no reason for anyone ever wanting to keep only the 4 most recent drafts, or, even weirder, the 4 most recent drafts-or-archives

ps: my bad, reading the code it seems that the number of versions to keep is not a sum, which makes a lot more sense. Maybe the help text could be improved a bit to explain this more clearly ?

gggeek · 2019-05-17T08:29:30Z

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php

+    const VERSION_DRAFT = 'draft';
+    const VERSION_ARCHIVED = 'archived';
+    const VERSION_PUBLISHED = 'published';
+    const VERSION_ALL = 'all';


nitpick: replace 'all' with something akin to 'all-except-published'

gggeek · 2019-05-17T08:34:04Z

Q: does ez5 have a mechanism to automatically purge drafts older than X days? If not, it would be a nice addition. Adding an option to this command to operate on a basis of last-modified besides the max-items to keep would allow it to be used for such scenarios

kmadejski requested review from adamwojs, andrerom, alongosz and vidarl August 29, 2018 13:02

adamwojs requested changes Aug 29, 2018

View reviewed changes

kmadejski changed the title ~~[WIP] EZP-29451: As an Administrator I want to have a CLI command for database cleanup~~ EZP-29451: As an Administrator I want to have a CLI command for database cleanup Sep 4, 2018

kmadejski added the Ready for review label Sep 4, 2018

kmadejski force-pushed the ezp_29451 branch from 7337b0d to df74d62 Compare September 5, 2018 09:09

vidarl approved these changes Sep 5, 2018

View reviewed changes

kmadejski force-pushed the ezp_29451 branch 2 times, most recently from fbc534b to 5a90c66 Compare September 13, 2018 13:56

alongosz reviewed Sep 13, 2018

View reviewed changes

kmadejski force-pushed the ezp_29451 branch from 582ac51 to f843198 Compare September 14, 2018 10:42

ezsystems deleted a comment from ezrobot Sep 14, 2018

kmadejski requested a review from konradoboza September 14, 2018 10:43

konradoboza approved these changes Sep 14, 2018

View reviewed changes

eZ/Bundle/EzPublishCoreBundle/Command/RemoveVersionsCommand.php Outdated Show resolved Hide resolved

alongosz approved these changes Sep 14, 2018

View reviewed changes

kmadejski added Ready for QA and removed Ready for review labels Sep 14, 2018

kmadejski added 6 commits September 17, 2018 14:19

As an Administrator I want to have a CLI command for database cleanup

aaf8d6e

fixup! As an Administrator I want to have a CLI command for database …

6bf283d

…cleanup

fixup! fixup! As an Administrator I want to have a CLI command for da…

ed4127c

…tabase cleanup

fixup! fixup! fixup! As an Administrator I want to have a CLI command…

dbe72d4

… for database cleanup

Changed command name

4d9c61c

Adapt command for 6.7

23e9fb7

konradoboza approved these changes Sep 17, 2018

View reviewed changes

andrerom approved these changes Sep 17, 2018

View reviewed changes

adamwojs approved these changes Sep 18, 2018

View reviewed changes

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php Outdated Show resolved Hide resolved

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php Outdated Show resolved Hide resolved

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php Show resolved Hide resolved

Review improvements

1726cc0

kmadejski added Ready for QA and removed Ready for review labels Sep 19, 2018

micszo self-assigned this Oct 3, 2018

Fixed invalid behavior of version counter

a226127

andrerom reviewed Dec 17, 2018

View reviewed changes

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php Show resolved Hide resolved

kmadejski added 2 commits December 18, 2018 10:25

Improved mapping of version status

d75927b

fixup! Improved mapping of version status

9de4eee

andrerom reviewed Dec 18, 2018

View reviewed changes

eZ/Bundle/EzPublishCoreBundle/Command/CleanupVersionsCommand.php Outdated Show resolved Hide resolved

ezsystems deleted a comment from ezrobot Dec 18, 2018

fixup! fixup! Improved mapping of version status

3eab499

andrerom approved these changes Dec 18, 2018

View reviewed changes

micszo assigned m-tyrala and unassigned micszo Dec 19, 2018

Covered edge-case with keep = 0

1ada6b8

m-tyrala approved these changes Dec 19, 2018

View reviewed changes

m-tyrala added QA approved and removed Ready for QA labels Dec 19, 2018

m-tyrala removed their assignment Dec 19, 2018

lserwatka merged commit b74cd70 into ezsystems:6.7 Dec 19, 2018

gggeek reviewed May 17, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-29451: As an Administrator I want to have a CLI command for database cleanup #2431

EZP-29451: As an Administrator I want to have a CLI command for database cleanup #2431

kmadejski commented Aug 29, 2018 •

edited

Loading

adamwojs commented Aug 29, 2018

kmadejski commented Sep 4, 2018

kmadejski commented Sep 5, 2018

vidarl left a comment •

edited

Loading

kmadejski commented Sep 13, 2018

kmadejski commented Sep 14, 2018 •

edited

Loading

konradoboza left a comment

kmadejski commented Dec 17, 2018

kmadejski commented Dec 19, 2018

m-tyrala left a comment

lserwatka commented Dec 19, 2018

gggeek May 17, 2019

gggeek May 17, 2019

gggeek May 17, 2019

gggeek May 17, 2019

gggeek commented May 17, 2019

EZP-29451: As an Administrator I want to have a CLI command for database cleanup #2431

EZP-29451: As an Administrator I want to have a CLI command for database cleanup #2431

Conversation

kmadejski commented Aug 29, 2018 • edited Loading

adamwojs commented Aug 29, 2018

kmadejski commented Sep 4, 2018

kmadejski commented Sep 5, 2018

vidarl left a comment • edited Loading

Choose a reason for hiding this comment

kmadejski commented Sep 13, 2018

kmadejski commented Sep 14, 2018 • edited Loading

konradoboza left a comment

Choose a reason for hiding this comment

kmadejski commented Dec 17, 2018

kmadejski commented Dec 19, 2018

m-tyrala left a comment

Choose a reason for hiding this comment

lserwatka commented Dec 19, 2018

gggeek May 17, 2019

Choose a reason for hiding this comment

gggeek May 17, 2019

Choose a reason for hiding this comment

gggeek May 17, 2019

Choose a reason for hiding this comment

gggeek May 17, 2019

Choose a reason for hiding this comment

gggeek commented May 17, 2019

kmadejski commented Aug 29, 2018 •

edited

Loading

vidarl left a comment •

edited

Loading

kmadejski commented Sep 14, 2018 •

edited

Loading