Bug 42476 - [PATCH] Bugfix for XMLReader: Support for other ImageReaders
Summary: [PATCH] Bugfix for XMLReader: Support for other ImageReaders
Status: CLOSED FIXED
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: images (show other bugs)
Version: trunk
Hardware: All All
: P2 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-21 12:16 UTC by Max Berger
Modified: 2012-04-01 06:32 UTC (History)
0 users



Attachments
Patch for XMLReader (2.26 KB, patch)
2007-05-21 12:16 UTC, Max Berger
Details | Diff
Alternate Patch for ImageReaders (6.19 KB, patch)
2007-05-21 13:50 UTC, Max Berger
Details | Diff
Comprehensive Patch for Image Readers (15.75 KB, patch)
2007-05-22 01:40 UTC, Max Berger
Details | Diff
Yet another try (6.24 KB, patch)
2007-05-23 02:07 UTC, Max Berger
Details | Diff
Patch with .close (7.07 KB, patch)
2007-05-27 01:10 UTC, Max Berger
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Max Berger 2007-05-21 12:16:12 UTC
XMLReader has serveral bugs stopping any other reader to fire afterwards. This is particularly annoying if 
one tries to add another Image Reader (such as the ODF-Reader for the JEuclid-Plugin :)

This patch fixes:
- A null pointer exception
- Stream Closure
- Reduces warning to debug statement
Comment 1 Max Berger 2007-05-21 12:16:53 UTC
Created attachment 20227 [details]
Patch for XMLReader

the actual patch
Comment 2 Max Berger 2007-05-21 12:49:59 UTC
Just figured out a better place to put the Instream wrapper:

org.apache.fop.image.analyser.ImageReaderFactory

after all, no image reader should be able to close the input stream, and every image reader should start 
with a fresh (=reset) input stream.
Comment 3 Max Berger 2007-05-21 13:50:29 UTC
Created attachment 20228 [details]
Alternate Patch for ImageReaders

This is an alternate patch, with the no-close stream adapter moved to
ImageFactory.
Pick which one you like better :)
Comment 4 Max Berger 2007-05-22 01:16:34 UTC
And I have yet another idea: The "reset" functionality should also be moved out
into the ImageReaderFactory (patch forthcoming)
Comment 5 Max Berger 2007-05-22 01:40:55 UTC
Created attachment 20239 [details]
Comprehensive Patch for Image Readers

This is a more comprehensive patch, which solves the bugs shown in the first
patch, and does some refactoring to make the implementation of future
ImageReaders easier.

While creating this patch, I notices that there is a LOT of functionality
duplicated in the image readers, this should be cleaned up at some point.

I promise this will be the last version of my patch on this issue :)
Comment 6 Vincent Hennebert 2007-05-23 00:59:18 UTC
(In reply to comment #5)

Hi Max,

> Created an attachment (id=20239) [edit]
> Comprehensive Patch for Image Readers
> 
> This is a more comprehensive patch, which solves the bugs shown in the first
> patch, and does some refactoring to make the implementation of future
> ImageReaders easier.

Thanks for your patch. Unfortunately it breaks the testsuite... Did you run it
before submitting the patch? Use "ant junit" to check that nothing is broken.

Also, please checkstyle your code before submitting your patch (some files
contained forbidden tab characters). We have a checkstyle-4.0.xml at the root of
the project.

Can you please check what is not working, and submit a new patch?

Thanks,
Vincent
Comment 7 Max Berger 2007-05-23 02:07:29 UTC
Created attachment 20244 [details]
Yet another try

Ok, installed ant-junit (all tests pass), and enabled checkstyle (no new issues
introduced). Also went back to a more "conservative" approach.

There is still a lot of duplicated functionality in the Image Readers, which at
some point should be cleaned up.
Comment 8 Max Berger 2007-05-27 01:10:38 UTC
Created attachment 20278 [details]
Patch with .close

Added Closing of Image Streams, otherwise same as before
Comment 9 Vincent Hennebert 2007-05-29 00:52:12 UTC
(In reply to comment #8)
> Created an attachment (id=20278) [edit]
> Patch with .close
> 
> Added Closing of Image Streams, otherwise same as before

OK, patch applied now. Thanks for your perseverance, Max.

I moved the UnclosableInputStream class to the o.a.fop.util package, where I
think it's better placed.
I'm ok with reducing the log level from warning to debug in SVGReader, as SVG
errors will still be reported (BridgeException). Only regular I/O errors won't
appear, which I guess is ok for now.

Vincent
Comment 10 Glenn Adams 2012-04-01 06:32:43 UTC
batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed