-
Notifications
You must be signed in to change notification settings - Fork 34
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
Modify NitfReading classes to support closing / try with resources #191
base: master
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a unit test case, and one minor format thing, otherwise looks good.
@dcruver Any thoughts or concerns?
@@ -125,4 +125,10 @@ public final void skip(final long count) throws NitfFormatException { | |||
throw new NitfFormatException(GENERIC_READ_ERROR_MESSAGE + ex.getMessage(), numBytesRead); | |||
} | |||
} | |||
|
|||
@Override public void close() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Override
would be better on a separate line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vaguely remember us running into a problem with closing this stream because it was being passed around within Alliance. I think that since we've added .end()
to the fluent classes, we might be ok to use this, now. We should verify that it doesn't cause a problem outside of imaging-nitf, however.
b77a871
to
811f204
Compare
Codecov Report
@@ Coverage Diff @@
## master #191 +/- ##
============================================
+ Coverage 88.14% 88.15% +<.01%
- Complexity 2068 2069 +1
============================================
Files 199 199
Lines 6309 6314 +5
Branches 494 494
============================================
+ Hits 5561 5566 +5
+ Misses 567 566 -1
- Partials 181 182 +1 |
I ran into a locked file bug that stemmed from this module while working on integration tests for another project. The problem starts here, a
NitfInputStreamReader
is created with a newFileInputStream
object inside of it. There is no way to close thatFileInputStream
object other than relying on garbage collection which leads to bugs. This PR is to make the miscellaneous nitf parsing objects closeable to prevent these bugs.