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

GORA-467 Flushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream #55

Merged
merged 1 commit into from
Feb 5, 2016

Conversation

emopers
Copy link
Contributor

@emopers emopers commented Feb 4, 2016

When a DataOutputStream instance wraps an underlying ByteArrayOutputStream instance,
it is recommended to flush or close the DataOutputStream before invoking the underlying instances's toByteArray(). Also, it is a good practice to call flush/close explicitly as mentioned for example here.
This pull request add a flush method before toByteArray. I have also added a Jira Issue.

@emopers emopers changed the title GORA-467 Flushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStreamFlushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream GORA-467 Flushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream Feb 4, 2016
@lewismc
Copy link
Member

lewismc commented Feb 4, 2016

Hi, thanks for the pull request. Are there any other instances of this behavior throughout the codebase? If so it would be real nice to catch them all at the same time.

@lewismc
Copy link
Member

lewismc commented Feb 4, 2016

Hi @emopers I just undertook a quick scan of the codebase for ByteArrayOutputStream, I found the following instances. Can you please check these out as well?

./gora-accumulo/src/main/java/org/apache/gora/accumulo/encoders/BinaryEncoder.java
./gora-accumulo/src/main/java/org/apache/gora/accumulo/store/AccumuloStore.java
./gora-accumulo/src/main/java/org/apache/gora/accumulo/util/FixedByteArrayOutputStream.java
./gora-accumulo/src/test/java/org/apache/gora/accumulo/store/PartitionTest.java
./gora-cassandra/src/main/java/org/apache/gora/cassandra/serializers/AvroSerializerUtil.java
./gora-core/src/main/java/org/apache/gora/util/AvroUtils.java
./gora-core/src/main/java/org/apache/gora/util/IOUtils.java
./gora-core/src/test/java/org/apache/gora/util/TestWritableUtils.java
./gora-hbase/src/main/java/org/apache/gora/hbase/util/HBaseByteInterface.java

@emopers
Copy link
Contributor Author

emopers commented Feb 5, 2016

@lewismc Alright, I will look into these and let you know.

@lewismc
Copy link
Member

lewismc commented Feb 5, 2016

Thanks @emopers

@emopers
Copy link
Contributor Author

emopers commented Feb 5, 2016

@lewismc I just found two classes out the above you mentioned, which were not following the contract. All other classes looks good.
I have made more commits and squashed them into one. Let me know if can further help.

@lewismc
Copy link
Member

lewismc commented Feb 5, 2016

@emopers thanks great, I will merge thus patch.
BTW, on another note, I noticed that there a bunch of 'similar' issues present within gora-accumulo's BinaryEncoder, the issues as specifically highlighted by the Sonar Analysis we have running after builds. If you are interested in fixing these issues (marked as critical by Sonar), it would be ideal.

@asfgit asfgit merged commit 4f98503 into apache:master Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants