Inconsistent behaviour of /ingest/addMediaPackage

Steps to reproduce

Ingest a new recording using:
curl --digest -u digestuser:digestpasswd -H "X-Requested-Auth: Digest" 'http://localhost:8080/ingest/addMediaPackage/<WorkFlowId>' -F 'flavor=presentation/source' -F 'title=testingestviacurl7' -F "BODY=@test.mkv"
This will work.

Try to ingest another one using the same fields, but change the order:
curl --digest -u digestuser:digestpasswd -H "X-Requested-Auth: Digest" 'http://localhost:8080/ingest/addMediaPackage/<WorkFlowId>' -F "BODY=@test.mkv" -F 'flavor=presentation/source' -F 'title=testingestviacurl9'
This will fail with "org.opencastproject.workflow.api.WorkflowOperationException: Media package does not meet criteria for publication". The reason seems to be that the file will be uploaded but won't be saved and added to the mediapackage.

Possible solutions:

  • Change the docs, make clear that the BODY fields has to be the last one and let the ingest fail otherwise.

  • Make the order irrelevant.

Additional note: The documentation states that there are “Required (form) params“ and a “Body (upload) param”. This is really confusing as the “Body (upload) param” is also a form parameter with the name “BODY”.

Activity

Show:
Lars Kiesow
May 3, 2013, 2:12 PM
Edited

I know, it is probably the thinking of a C programmer, but is it not possible in Java to preserve something like a pointer to the data from the file until the other data were read, too?

If not, we should at least change the documentation accordingly.

Rubén Pérez
May 3, 2013, 4:21 PM
Edited

I've seen this issue with the Apache libraries and it's hard to avoid. However, the old implementation did not use those libraries at all, which made the service agnostic to the order of the POST parameters.

Of course this has nothing to do with your patch, Lars (I never said there was anything wrong with it, by the way). In fact, after checking the relevant file more closely, I've seen that it doesn't even change the addZippedMediapackage endpoint. I just thought the description of this issue was general enough to cover both inconsistencies in the Ingest Service. Now that I have looked at this more closely, I agree that perhaps this other issue deserves its own ticket, not only because it seems hard to fix, but because IT COMES FROM ANOTHER MODIFICATION OF THE API WITHOUT EVEN NOTIFY THE OTHER DEVELOPERS.

I am fed up of complaining on list and in the devOps meetings that we should keep the our APIs and data stable between versions, and if they need to evolve to incorporate new functionalities or improve the existing ones, we should observe a strict "adaptation" period where the old features/syntax are deprecated and such state duly notified to the users and/or vendors so that they can gradually adapt to the new features/syntax. That's what normal, proffesional programs do. Of course, as it turns out, Matterhorn is not any of those.

EDIT: In order to keep your patch separated from this other issue, how does it sound we change this ticket's description to be more specific ("inconsistent behaviour of the endpoint addMediapackage in the Ingest Service"), so that you patch can be applied (since I believe it fixes the intended behaviour of that endpoint) and create a new ticket for this last issue?

Lars Kiesow
May 3, 2013, 10:52 PM

Ruben & Benjamin: You are right and I was wrong. It does not work and it is not easily fixable. After some research I would say that there are three options for the /addZippedMediapackage:
1. Keep it like it is but change the documentation accordingly and return a 400 Bad Request if the order is wrong.
2. Do something like this: http://gregorbowie.wordpress.com/2010/10/13/apache-commons-file-upload-order-dependency/. Of cause we cannot use a ByteArrayOutputStream as the files we have to handle might be huge. So we could write it to disk if the other parameters were not set before and than later on read the file again. This is easy to implement but is quite inefficient.
3. The third way would be to split the addZippedMediaPackage method from IngestServiceImpl.java into one method which takes the zipped mediapackage, extrackts all media files to the working directory etc. and one to do the actual ingest.

@Ruben: I agree with you that the APIs should not be changed without to be compartible for a while and especially not without notice.

To seperate both issues I created a new issue for /addZippedMediapackage. I will also move the relevant comments to the new issue:
https://opencast.jira.com/browse/MH-9634

Rubén Pérez
May 4, 2013, 12:58 PM

Thank you Lars!

Greg Logan
May 7, 2013, 7:48 PM

Merged into 1.4.x with rev 14543.

Fixed and reviewed

Assignee

Lars Kiesow

Reporter

Lars Kiesow

Severity

Incorrectly Functioning With Workaround

Tags (folksonomy)

Components

Fix versions

Affects versions

Priority

Blocker
Configure