Re: [compress] Need Feedback for COMPRESS-479
On 2019-08-14, Matt Sicker wrote: > Yes, I think you understand us. A strategy pattern with default sensible > strategies to choose. Done now. I'm going to tweak the implementation a little more but the API of ExtraFieldParsingBehavior seems to work quite well. Thanks Stefan - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [compress] Need Feedback for COMPRESS-479
We all understood each other in a brief email thread, remarkable! :-) Gary On Wed, Aug 14, 2019 at 9:00 AM Matt Sicker wrote: > Yes, I think you understand us. A strategy pattern with default sensible > strategies to choose. > > On Wed, Aug 14, 2019 at 06:08, Stefan Bodewig wrote: > > > On 2019-08-13, Matt Sicker wrote: > > > > > The enum makes sense. Are there any feasible ways to, say, configure > > > some sort of handler class that can implement logic around unknown > > > fields? > > > > Not really. The only extension point here currently is plugging in your > > own implementations of ZipExtraField via the static > > ExtraFieldUtils.register - which could use some ServiceLoader magic one > > day :-) > > > > IIUC you and Gary are both saying the same thing. The enum values are > > sensible defaults but it would be good to provide a way to do the same > > things with custom code (callbacks or interface implementations). > > > > It should be possible to split ExtraFieldParsingMode into a strategy > > pattern interface implemented by the enum providing default > > implementations. This may also reduce some other implementation quirks > > (I'm not too happy with the current exception handler inside > > mergeExtraFields). > > > > Thanks! > > > > Stefan > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > -- > Matt Sicker >
Re: [compress] Need Feedback for COMPRESS-479
Yes, I think you understand us. A strategy pattern with default sensible strategies to choose. On Wed, Aug 14, 2019 at 06:08, Stefan Bodewig wrote: > On 2019-08-13, Matt Sicker wrote: > > > The enum makes sense. Are there any feasible ways to, say, configure > > some sort of handler class that can implement logic around unknown > > fields? > > Not really. The only extension point here currently is plugging in your > own implementations of ZipExtraField via the static > ExtraFieldUtils.register - which could use some ServiceLoader magic one > day :-) > > IIUC you and Gary are both saying the same thing. The enum values are > sensible defaults but it would be good to provide a way to do the same > things with custom code (callbacks or interface implementations). > > It should be possible to split ExtraFieldParsingMode into a strategy > pattern interface implemented by the enum providing default > implementations. This may also reduce some other implementation quirks > (I'm not too happy with the current exception handler inside > mergeExtraFields). > > Thanks! > > Stefan > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > -- Matt Sicker
Re: [compress] Need Feedback for COMPRESS-479
On 2019-08-13, Matt Sicker wrote: > The enum makes sense. Are there any feasible ways to, say, configure > some sort of handler class that can implement logic around unknown > fields? Not really. The only extension point here currently is plugging in your own implementations of ZipExtraField via the static ExtraFieldUtils.register - which could use some ServiceLoader magic one day :-) IIUC you and Gary are both saying the same thing. The enum values are sensible defaults but it would be good to provide a way to do the same things with custom code (callbacks or interface implementations). It should be possible to split ExtraFieldParsingMode into a strategy pattern interface implemented by the enum providing default implementations. This may also reduce some other implementation quirks (I'm not too happy with the current exception handler inside mergeExtraFields). Thanks! Stefan - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [compress] Need Feedback for COMPRESS-479
Hi Stefan, I like having predefined behaviors in the enum but it does not seem to allow for extension. I am wondering if we would be better served by using an interface invoked by a callback mechanism where predefined implementations exist for the enum values you now have. I would see in addition, a SystemOut impl that printfs these extra fields we do not handle. Gary On Tue, Aug 13, 2019 at 3:01 PM Stefan Bodewig wrote: > Hi > > https://issues.apache.org/jira/browse/COMPRESS-479 highlights a problem > where our zip reading code may reject an archive because it uses extra > fields that we only partially understand. In this case the archive is > not a real one, but it is easy to envision extra fields in the wild that > use more advanced versions than we currently support. > > For most of our users the ZIP extra fields will be noise and they really > are only interested in the actual content of the archives. > > Therefore I have decided to treat any such "not quite as we expect" > extra field the same way as any "we have no idea how this extra field > looks internally" field - i.e. just store it but don't try to parse > it. > > In addition I have provided an API inside of ZipArchiveEntry that allows > users to specify in detail how strict they want the extra field parsing > to be. > > Does this make sense to you? Would you recommend taking a different > approach? And it if makes sense, are the enum names I've chosen in > > https://github.com/apache/commons-compress/blob/2bf678bbc6c6002569559b90ea29a031f035f067/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java#L1117 > understandable? > > Thanks > > Stefan > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
Re: [compress] Need Feedback for COMPRESS-479
The enum makes sense. Are there any feasible ways to, say, configure some sort of handler class that can implement logic around unknown fields? Sort of an extensibility model there, but hopefully without having to extend ZipInputStream or anything like that. On Tue, 13 Aug 2019 at 14:01, Stefan Bodewig wrote: > > Hi > > https://issues.apache.org/jira/browse/COMPRESS-479 highlights a problem > where our zip reading code may reject an archive because it uses extra > fields that we only partially understand. In this case the archive is > not a real one, but it is easy to envision extra fields in the wild that > use more advanced versions than we currently support. > > For most of our users the ZIP extra fields will be noise and they really > are only interested in the actual content of the archives. > > Therefore I have decided to treat any such "not quite as we expect" > extra field the same way as any "we have no idea how this extra field > looks internally" field - i.e. just store it but don't try to parse > it. > > In addition I have provided an API inside of ZipArchiveEntry that allows > users to specify in detail how strict they want the extra field parsing > to be. > > Does this make sense to you? Would you recommend taking a different > approach? And it if makes sense, are the enum names I've chosen in > https://github.com/apache/commons-compress/blob/2bf678bbc6c6002569559b90ea29a031f035f067/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java#L1117 > understandable? > > Thanks > > Stefan > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > -- Matt Sicker - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
