Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is more than one archive document allowed on p:archive? #295

Closed
ndw opened this issue Dec 15, 2019 · 9 comments
Closed

Is more than one archive document allowed on p:archive? #295

ndw opened this issue Dec 15, 2019 · 9 comments
Assignees

Comments

@ndw
Copy link
Collaborator

@ndw ndw commented Dec 15, 2019

The archive port (like the manifest port) is a sequence because empty is allowed. The description of the manifest port says explicitly that it's an error if more than one manifest document is provided. The description of archive doesn't. Should it? If not, what does it mean to provide several archives?

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Dec 15, 2019

There might be an operation to merge archives, therefore I wouldn’t make multiple archives on the archive port an error unconditionally.

@ndw

This comment has been minimized.

Copy link
Collaborator Author

@ndw ndw commented Dec 15, 2019

Ah, fair point. But for ZIP file operations, we should raise an error, agreed?

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Dec 15, 2019

It’s certainly an uncommon use case for zip archives, but apparently they can be merged efficiently. So if some implementation chooses to provide a zip merge, we maybe shouldn’t make it an error. On the other hand, we shouldn’t require applications to support merge. We currently limit the allowed zip operations to update, create, freshen, and delete. For these operations, we say that there must be zero or one (exactly one for delete) documents on the archive port. In the interest of finishing the steps, I would leave it as it is now (that is, do not allow other zip operations). If your question aims at whether we should define new error codes for these four zip operations when there are too many documents on the archive port: I think we should. But we probably shouldn’t make it an error if a pipeline author uses an operation on zip files that is not contained in the list above. This way, implementations can provide their own operations without overtly violating the spec.

@ndw

This comment has been minimized.

Copy link
Collaborator Author

@ndw ndw commented Dec 16, 2019

In as much as create and freshen say that they behave exactly like update, I suppose the spec says there can be at most one archive, but I think that could be clarified.

Proposal:

  1. Clarify that at most one ZIP file is allowed for the zip commands we identify.
  2. Add an explicit error for the case where more than one zip is supplied.
  3. Since I'll be touching the spec, add a note that says additional commands are implementation-defined
@ndw ndw self-assigned this Dec 16, 2019
@xml-project

This comment has been minimized.

Copy link
Contributor

@xml-project xml-project commented Dec 18, 2019

What is wrong with XC0080? Obviously I missed the point because I thought this error is just for the case discussed.

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Dec 18, 2019

It is, but I think Norm wants to be more specific if the format is zip.

Is it allowed for a processor to add explanatory text to error messages? Then it can raise C0080 and add: For Zip archives, the number of archives is exactly one for the delete command, and zero or one for the create, update, and freshen commands.

@xml-project

This comment has been minimized.

Copy link
Contributor

@xml-project xml-project commented Dec 18, 2019

Is it allowed for a processor to add explanatory text to error messages?

Why not?

Then it can raise C0080 and add: For Zip archives, the number of archives is exactly one for the delete command, and zero or one for the create, update, and freshen commands.

This is exactly what I implemented.

@ndw

This comment has been minimized.

Copy link
Collaborator Author

@ndw ndw commented Dec 18, 2019

I think XC0080 is just fine. I may simply have overlooked that before I opened this issue. I still think it's a bit odd that delete says only one ZIP archive may appear when, AFAICT, only one may appear for any of the zip commands.

So I still think points 1 and 3 from my third comment hold.

@ndw

This comment has been minimized.

Copy link
Collaborator Author

@ndw ndw commented Dec 21, 2019

Fixed by my fix for #294, I think.

@ndw ndw closed this Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.