A storage error doesn't prevent the creation of the File or Item records

[zerocrates]

If you create a new Item, choose some metadata, and choose a file to upload, we seem to get the following if there's some error when storing the file:

• The Item record is created, but has none of its Element metadata
• The File record is created
• The underlying file itself is, obviously, not stored.

This is under Synchronous job dispatching. I suspect that the Element metadata would be successfully saved under one of the "true" background processing handlers if there was a storage problem, since the two are separate.

I'm unsure if the whole File record is created as part of the upload job, if so, transaction support could solve this for us (though not the Item part of it, since these would be separate db sessions).

Even under the current situation, it seems like we could defer saving the File record until the actual files have been stored.

(Stopping the Item creation for a file issue might not be necessary or desired, but the current synchronous behavior of resulting in an empty Item is no good.)

[zerocrates]

The File record appears to be saved before we kick off the background job, and the data there is used by the job.

Not sure if its feasible to shift the File creation entirely into the job and just pass the appropriate data, but that'd be one solution.

The job could also delete the File record upon an error, but that's a little more fragile than simply not saving it at all until everything's a-ok.

[kriskelly]

The most robust solution for this would be to add transaction support, which is itself blocked by the lack of a baked-in search alternative to MyISAM fulltext search.

Storage exceptions are thrown in File::afterInsert(), so wrapping that part of Omeka_Record::save() in a try/catch to delete the record might also work, though seems hackish.

[kriskelly]

Moving all file processing into a job could also work, though that will take some tinkering.

[zerocrates]

The most robust solution for this would be to add transaction support, which is itself blocked by the lack of a baked-in search alternative to MyISAM fulltext search.

Storage exceptions are thrown in File::afterInsert(), so wrapping that part of Omeka_Record::save() in a try/catch to delete the record might also work, though seems hackish.

Both transaction support and a try/catch on afterInsert wouldn't help us
when the exception is being thrown in a different process, right?

Those kinds of things could help out in the currently-bad synchronous
case, but there might be some trivial reordering that could be done to
bring synchronous behavior into line with the better but still bad
current background behavior.

[kriskelly]

Oh yeah, you're right. Probably best to try/catch storage exceptions in the job and delete the file if necessary.

In this case, we're not worried about saving items, are we? The item doesn't depend on the file, so it wouldn't make sense to delete it if the file failed to upload.

On Jul 19, 2011, at 3:16 PM, zerocrates wrote:

On 07/19/2011 03:09 PM, kbkelly wrote:

The most robust solution for this would be to add transaction support, which is itself blocked by the lack of a baked-in search alternative to MyISAM fulltext search.

Storage exceptions are thrown in File::afterInsert(), so wrapping that part of Omeka_Record::save() in a try/catch to delete the record might also work, though seems hackish.

Both transaction support and a try/catch on afterInsert wouldn't help us
when the exception is being thrown in a different process, right?

Those kinds of things could help out in the currently-bad synchronous
case, but there might be some trivial reordering that could be done to
bring synchronous behavior into line with the better but still bad
current background behavior.

Reply to this email directly or view it on GitHub:
#88 (comment)

[zerocrates]

On 07/19/2011 03:21 PM, kbkelly wrote:

In this case, we're not worried about saving items, are we? The item doesn't depend on the file, so it wouldn't make sense to delete it if the file failed to upload.

I think that's right. The only snag is that, thanks to the order things
get saved in, when there's a storage error and you're using the
synchronous dispatcher, the Item record gets saved, but the element
texts never do, so you end up with an incomplete Item. I think this is
fixable just by making sure that the Item saving process does it's thing
before we kick off the File job.

[kriskelly]

Sounds good to me.

[zerocrates]

So, the tricky part about reordering the save order is that element texts get saved by ActsAsElementText, while the files are saved by Item::afterSave, and Omeka_Record always runs the record's own callback first.

[patrickmj]

should probably keep the item in place to not lose work