Skip to content

Conversation

@mdaniel
Copy link

@mdaniel mdaniel commented Nov 2, 2013

There are circumstances under which one does not have access to a File and it seems unnecessary to open an InputStream only to serialize it to a File which is then subsequently read back into memory due to the constructor Reader(File).

Thus, this PR adds a constructor to Reader that accepts a URL.

I briefly thought about going to the next logical conclusion and adding one for InputStream but I am not familiar enough with NIO to avoid building up a List<byte> since I would not know the length of the incoming stream.

I will also submit a PR for com.maxmind.geoip2.DatabaseReader over in GeoIP2-java to expose the new functionality.

Matthew L Daniel added 3 commits November 1, 2013 18:07
These objects are in the same package as the Test, and thus do not require import.
Updated the test to exercise both the File and the URL flavors.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7866b3c on mdaniel:read-from-url into e162bfb on maxmind:master.

Since this project is Open Source, it is very handy to have Maven
download the sources for the jars found in central.

Some projects also attach javadoc, but the sources provide javadoc
(at least with IJ) so I find that they just take up space.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, this can return -1, which will cause the next line to fail

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had forgotten about that. However, if we go with the BAOS mechanism you mention below, then we won't need content-length and this bug goes away.

Matthew L Daniel added 2 commits November 4, 2013 10:36
InputStream is the common denominator and contains enough information to
construct the ThreadBuffer (and thus the Reader).
Those warnings in IJ hide potentially real bugs.
@mdaniel
Copy link
Author

mdaniel commented Nov 4, 2013

I switched to the BAOS as we discussed and (of course) all the tests pass.

I'll update the GeoIP2-java PR in a second.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0cc4dfb on mdaniel:read-from-url into e162bfb on maxmind:master.

@ghost ghost merged commit 0cc4dfb into maxmind:master Nov 4, 2013
@oschwald
Copy link
Member

oschwald commented Nov 4, 2013

Thanks! 👍

@mdaniel mdaniel deleted the read-from-url branch November 4, 2013 21:01
TuxCoding referenced this pull request in TuxCoding/MaxMind-DB-Reader-java Mar 15, 2018
Add the missing cache files
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants