[MGNLDAM-606] Error restoring a deleted asset to its previous version Created: 27/Jul/15  Updated: 07/Jul/16  Resolved: 04/Jul/16

Status: Closed
Project: Magnolia DAM Module
Component/s: DAM App, User Interaction
Affects Version/s: 2.0.9, 2.1
Fix Version/s: 2.0.14, 2.1.6

Type: Bug Priority: Neutral
Reporter: Richard Gange Assignee: Robert Šiška
Resolution: Fixed Votes: 3
Labels: support
Remaining Estimate: 0d
Time Spent: 2d 0.75h
Original Estimate: Not Specified

Issue Links:
Relates
relates to MGNLDAM-664 Thumbnail preview doesn't refresh aft... Closed
relates to MAGNOLIA-6702 Add switch for restoring including de... Closed
causality
caused by MGNLDAM-557 Restore including subnodes must be po... Closed
dependency
depends upon MGNLUI-3921 Adjust restore previous version actio... Closed
duplicate
is duplicated by MGNLDAM-654 Assets: restore previous version thro... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Kromeriz 49, Kromeriz 51
Story Points: 5

 Description   

After deleting an asset, an editor is given the option to "Restore previous version". Performing this action restores the asset but throws an error.

2015-07-27 13:50:19,139 INFO  info.magnolia.module.scheduler.CommandJob         : Starting job [UI Action triggered execution of [default:markAsDeleted] by user [superuser]. (3)]...
2015-07-27 13:50:19,257 INFO  info.magnolia.module.scheduler.CommandJob         : Job executed successfully [UI Action triggered execution of [default:markAsDeleted] by user [superuser]. (3)]
2015-07-27 13:50:22,145 INFO  info.magnolia.module.scheduler.CommandJob         : Starting job [UI Action triggered execution of [default:restorePreviousVersion] by user [superuser]. (4)]...
2015-07-27 13:50:22,198 ERROR info.magnolia.module.scheduler.CommandJob         : Cannot execute command restorePreviousVersion-default.
javax.jcr.UnsupportedRepositoryOperationException: Unable to perform a versioning operation on a non versionable node: /magnolia-logo-print/description_files
	at org.apache.jackrabbit.core.version.VersionManagerImplBase.checkVersionable(VersionManagerImplBase.java:293)
	at org.apache.jackrabbit.core.version.VersionManagerImplBase.getVersionHistory(VersionManagerImplBase.java:354)
	at org.apache.jackrabbit.core.VersionManagerImpl.access$700(VersionManagerImpl.java:73)
	at org.apache.jackrabbit.core.VersionManagerImpl$4.perform(VersionManagerImpl.java:184)
	at org.apache.jackrabbit.core.VersionManagerImpl$4.perform(VersionManagerImpl.java:180)
	at org.apache.jackrabbit.core.session.SessionState.perform(SessionState.java:216)
	at org.apache.jackrabbit.core.VersionManagerImpl.perform(VersionManagerImpl.java:96)
	at org.apache.jackrabbit.core.VersionManagerImpl.getVersionHistory(VersionManagerImpl.java:180)
	at org.apache.jackrabbit.core.NodeImpl.getVersionHistory(NodeImpl.java:2997)
	at info.magnolia.jcr.wrapper.DelegateNodeWrapper.getVersionHistory(DelegateNodeWrapper.java:267)
	at info.magnolia.jcr.wrapper.DelegateNodeWrapper.getVersionHistory(DelegateNodeWrapper.java:267)
	at info.magnolia.cms.core.version.BaseVersionManager.getAllVersions(BaseVersionManager.java:367)
	at info.magnolia.commands.impl.RestorePreviousVersionCommand.getPreviousVersion(RestorePreviousVersionCommand.java:141)
	at info.magnolia.commands.impl.RestorePreviousVersionCommand.restoreAllChildren(RestorePreviousVersionCommand.java:127)
	at info.magnolia.commands.impl.RestorePreviousVersionCommand.restore(RestorePreviousVersionCommand.java:100)
	at info.magnolia.commands.impl.RestorePreviousVersionCommand.execute(RestorePreviousVersionCommand.java:74)
	at info.magnolia.commands.MgnlCommand.executeSynchronized(MgnlCommand.java:81)
	at info.magnolia.commands.MgnlCommand.execute(MgnlCommand.java:70)
	at info.magnolia.module.scheduler.CommandJob.execute(CommandJob.java:135)
	at org.quartz.core.JobRunShell.run(JobRunShell.java:223)
	at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:549)


 Comments   
Comment by Mikaël Geljić [ 16/Jun/16 ]

Copying my comment on the duplicate issue here:

Okay, so first the jcr:content doesn't need to be a mix:versionable; it is copied correctly when versioning. So far so good.

Thanks rsiska for the folder insights again; however I'm not fond of duplicating the action either. This seems to me as much complicated as option #2, except you need actions, actionbar and availability configuration which are terribly verbose — and when this occurs in resources workspace, we're screwed and have to do it again.

What I don't understand is that we should be able to rely on the default rule properly: info.magnolia.commands.impl.RuleBasedCommand#getDefaultRule
It always has at least the mgnl:contentNode type, and adds the two mgnl:metaData and mgnl:resource. Again, all good when creating versions.

So isn't it the restore action not consuming this rule properly?

(And ideally, if our versioning were reading node-type definitions, we could just make sure to use the correct On-Parent-Version action for the jcr:content node, right in magnolia-dam-nodetypes.xml, but different story).

Comment by Robert Šiška [ 16/Jun/16 ]

Okay.
So what's the idea? Revert this and add rule delegation to the action?

That way we would have to change main, ui & dam. Also note, that this is originally from support ticket, so we need it at least on master & 5.3.x. (so that's, like, 6 PRs? phew.. )

On a related note: we noticed that now deprecated RestorePreviousVersionAction used to create a new version before restoring (the command doesn't do that).
If we want to bring this functionality back, we'd need to allow specifying two rules - one for restoring the previous version and one for creating version before restoring.
Should the command/action allow definition of both?

Comment by Robert Šiška [ 21/Jun/16 ]

Current solution:

  • Add includingChildren switch to RestorePreviousVersionCommand and read rule from command context (MAGNOLIA-6702)
  • Adjust restore previous version action definition to pass rule to the command (MGNLUI-3921)
  • Make two actions for restoring assets - one with children and one without (this ticket)
Comment by Mikaël Geljić [ 24/Jun/16 ]
  • Accidental pom reactor changes must be reverted (how did this even pass review?)
  • Would deserve concrete tests for the few cases here (including MGNLDAM-654)
  • MGNLDAM-654 is "fixed" regardless of allowedTypes configuration; I can change or drop them, no effect. => Why do we configure allowedTypes under the restorePreviousVersion action? (for restorePreviousVersionIncludingChildren it's ok)

EDIT: includingChildren does work, here's what you get for not restarting the app...
EDIT2: restorePreviousVersionIncludingChildren works on a folder without configuration at all too? o__O it puzzles me again what the issue really was then.

Comment by Robert Šiška [ 01/Jul/16 ]

ad edit2: restoring folders with children did work since MGNLDAM-557. It's restoring of assets which was broken by it.

Comment by Philip Mundt [ 06/Jul/16 ]

One thing I found when QAing the release of 5.3.15 was that the dam-image-provider, which uses the provided image generators caches the images and keeps the last image in cache even after restoring. To reproduce:

  1. Add image and create version
  2. Update image and create version
  3. Restore previous version
  4. See thumbnail still displaying the second version of the image

Created a followup issue: MGNLDAM-664

Generated at Mon Feb 12 05:01:29 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.