Filesize validation for extraction service

amelsmajic commented 3 years ago

On production we currently have the following error popping up:

An ActiveRecord::RecordInvalid occurred in background at 2021-04-20 15:25:02 +0200 :

  Validation failed: Upload too large
  /home/sapphire/sapphire/shared/bundle/ruby/2.6.0/gems/activerecord-4.2.10/lib/active_record/validations.rb:79:in `raise_record_invalid'

This happens during extraction:

/home/sapphire/sapphire/releases/20210307215355/app/services/submission_extraction_service.rb:23:in `perform!'

when the archive is being removed:

/home/sapphire/sapphire/releases/20210307215355/app/services/submission_extraction_service.rb:100:in `remove_archive_asset!'

because the following function is called:

/home/sapphire/sapphire/releases/20210307215355/app/models/submission_asset.rb:198:in `remove_from_submission_filesize'

which performs a validation during save:

/home/sapphire/sapphire/shared/bundle/ruby/2.6.0/gems/activerecord-4.2.10/lib/active_record/transactions.rb:291:in `save!'

The problem is that even though the extracted files are below the maximum upload size, during the validation of the submission both the extracted files and the zip are summed together and checked if below the maximum upload size which leads to the validation failing.

The exact line triggering the validation is here: https://github.com/matthee/Sapphire/blob/bfa299fbf2c39104a124d38aa685a113af7cf21e/app/models/submission_asset.rb#L198

My suggestion for a fix would be not validating if the submission_asset is a archive:

submission.save!(validate: !self.archive?)

Or even not validating at all here:

submission.save!(validate: false)

As we cannot be above the maximum upload size after removing a file. Open for other suggestions.

PrangerStefan commented 3 years ago


  validates :submitter, presence: true
  validates :submitted_at, presence: true
  validates :exercise, presence: true
  validates :student_group, uniqueness: { scope: [:exercise_id, :exercise_attempt_id] }, if: :student_group

  validate :upload_size_below_exercise_maximum_upload_size

I guess out of all the things we validate here only the last validation is related to this issue anyways, and as you said this should not trigger when deleting assets.

Could you open up a PR with these changes?

matthee commented 3 years ago

In app/services/submission_extraction_service.rb:25

        # needs to be reset, in order to allow correct file-size validation
        submission_asset.processed_size = 0
        submission_asset.save(validate: false)

Looks to me as if setting the processed_size is no longer sufficient. IMHO this is the place where we need to modify the code.

Sapphire-CM / Sapphire

Filesize validation for extraction service #521