Refactor archive loading
ClosedPublic

Authored by elvisangelaccio on Sep 19 2016, 11:06 AM.

Details

Summary

Ark currently "load" an archive by using Archive *Archive::create() first and then ListJob *archive->list(). If an archive property is read *before* list() is called, the archive is listed in the background in listIfNotListed(). This design is broken a causes a lot or problems, the biggest being:

  • The infamous loop of listjobs: T3296
  • ark --batch --dialog huge-archive.tar.xz takes forever to show the extraction dialog, because the archive is listed in the background but BatchExtract doesn't (and can't) know it.

This diff basically changes ListJob into a new LoadJob. Archives are loaded by calling the static LoadJob *Archive::load(). The actual archive instance can be retrieve after the job has finished listing the archive.

We also introduces two new jobs (CreateJob and BatchExtractJob) to simplify the porting of addtoarchive.cpp and batchextract.cpp. The latter job does listing + extraction at once.

One small downside is the handling of non-existing archives, which we can't load. This is needed when we create a new archive from the Archive->New action. We need to be able to use the old Archive::create() in this case, but at least we can call it createEmpty() which is what actually happens.

Test Plan
  • Make sure T3296 is fixed
  • ark --batch --dialog huge-archive.tar.xz
  • Make sure everything else still works :D

Diff Detail

Repository
R36 Ark
Lint
Lint Skipped
Unit
Unit Tests Skipped
elvisangelaccio retitled this revision from to Refactor archive loading.
elvisangelaccio updated this object.
elvisangelaccio edited the test plan for this revision. (Show Details)
elvisangelaccio set the repository for this revision to R36 Ark.
Restricted Application added a project: Ark. · View Herald TranscriptSep 19 2016, 11:06 AM
elvisangelaccio changed the visibility from "elvisangelaccio (Elvis Angelaccio)" to "Public (No Login Required)".
elvisangelaccio changed the edit policy from "elvisangelaccio (Elvis Angelaccio)" to "All Users".
elvisangelaccio updated this object.
elvisangelaccio added a subscriber: kde-utils-devel.
rthomsen requested changes to this revision.Sep 27 2016, 8:33 AM
rthomsen edited edge metadata.

This is a huge commit. I've briefly glanced through it, but didn't notice any errors as such. Tested opening/modifying various archive formats. Some comments:

  • I can confirm that the endless loop is gone :)
  • The extraction dialog now appears instantly when using the batch switch on a large archive. However, it is disabled (gray) until the listjob/loadjob completes. This is the intented behavior, right?
  • One thing I noticed: The KMessageWidget no longer disappears after adding a file to a new archive.
  • Why are the new jobs in another source file?
This revision now requires changes to proceed.Sep 27 2016, 8:33 AM
  • The extraction dialog now appears instantly when using the batch switch on a large archive. However, it is disabled (gray) until the listjob/loadjob completes. This is the intented behavior, right?

Yes. We should probably add some feedback for the users, but that requires changing the layout of the dialog, somehow.

  • One thing I noticed: The KMessageWidget no longer disappears after adding a file to a new archive.

Ah yes, I also noticed this. Will have to check what's going on.

elvisangelaccio edited edge metadata.

Now there is a single Job base class.

elvisangelaccio edited edge metadata.

Rebased on top of master.

rthomsen accepted this revision.Oct 2 2016, 1:39 PM
rthomsen edited edge metadata.

Ship it!

This revision is now accepted and ready to land.Oct 2 2016, 1:39 PM
This revision was automatically updated to reflect the committed changes.