Skip to content

Commit f643133

Browse files
oschwaldoschwald
committed
Cleaned up code and updated comments
1 parent 0b5c3ea commit f643133

4 files changed

Lines changed: 25 additions & 81 deletions

File tree

‎src/main/java/com/maxmind/maxminddb/Decoder.java‎

Lines changed: 19 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020

2121
final class Decoder {
2222

23-
private final boolean DEBUG;
23+
private final static boolean DEBUG = System.getenv().get(
24+
"MAXMIND_DB_DECODER_DEBUG") != null;
2425
// XXX - This is only for unit testings. We should possibly make a
2526
// constructor to set this
2627
boolean POINTER_TEST_HACK = false;
@@ -79,7 +80,6 @@ void setOffset(int offset) {
7980
this.pointerBase = pointerBase;
8081
this.threadBuffer = threadBuffer;
8182
this.objectMapper = new ObjectMapper();
82-
this.DEBUG = System.getenv().get("MAXMIND_DB_DECODER_DEBUG") != null;
8383
}
8484

8585
Result decode(int offset) throws IOException {
@@ -91,7 +91,7 @@ Result decode(int offset) throws IOException {
9191

9292
Type type = Type.fromControlByte(ctrlByte);
9393

94-
if (this.DEBUG) {
94+
if (Decoder.DEBUG) {
9595
Log.debug("Offset", String.valueOf(offset));
9696
Log.debugBinary("Control byte", ctrlByte);
9797
Log.debug("Type", type.name());
@@ -115,7 +115,7 @@ Result decode(int offset) throws IOException {
115115
if (type.equals(Type.EXTENDED)) {
116116
int nextByte = buffer.get();
117117

118-
if (this.DEBUG) {
118+
if (Decoder.DEBUG) {
119119
Log.debug("Next byte", nextByte);
120120
}
121121

@@ -136,11 +136,11 @@ Result decode(int offset) throws IOException {
136136
int size = sizeArray[0];
137137
offset = sizeArray[1];
138138

139-
if (this.DEBUG) {
139+
if (Decoder.DEBUG) {
140140
Log.debug("Size", String.valueOf(size));
141141
}
142142

143-
if (this.DEBUG) {
143+
if (Decoder.DEBUG) {
144144
Log.debug("Offset", offset);
145145
Log.debug("Size", size);
146146
}
@@ -158,7 +158,7 @@ private Result decodeByType(Type type, int offset, int size)
158158
case ARRAY:
159159
return this.decodeArray(size, offset);
160160
case BOOLEAN:
161-
return this.decodeBoolean(size, offset);
161+
return new Result(Decoder.decodeBoolean(size), offset);
162162
case UTF8_STRING:
163163
TextNode s = new TextNode(this.decodeString(size));
164164
return new Result(s, newOffset);
@@ -195,28 +195,18 @@ private Result decodeByType(Type type, int offset, int size)
195195
(1 << 19) + ((1) << 11), 0 };
196196

197197
private Result decodePointer(int ctrlByte, int offset) {
198-
199198
int pointerSize = ((ctrlByte >>> 3) & 0x3) + 1;
200-
201-
if (this.DEBUG) {
202-
Log.debug("Pointer size", String.valueOf(pointerSize));
203-
}
204-
205199
int base = pointerSize == 4 ? (byte) 0 : (byte) (ctrlByte & 0x7);
206-
207200
int packed = this.decodeInteger(base, pointerSize);
201+
long pointer = packed + this.pointerBase
202+
+ this.pointerValueOffset[pointerSize];
208203

209-
if (this.DEBUG) {
204+
if (Decoder.DEBUG) {
205+
Log.debug("Pointer size", String.valueOf(pointerSize));
210206
Log.debug("Packed pointer", String.valueOf(packed));
211207
Log.debug("Pointer base", this.pointerBase);
212208
Log.debug("Pointer value offset",
213209
this.pointerValueOffset[pointerSize]);
214-
}
215-
216-
long pointer = packed + this.pointerBase
217-
+ this.pointerValueOffset[pointerSize];
218-
219-
if (this.DEBUG) {
220210
Log.debug("Pointer to", String.valueOf(pointer));
221211
}
222212

@@ -260,7 +250,6 @@ private int decodeInteger(int base, int size) {
260250
}
261251

262252
static int decodeInteger(ByteBuffer buffer, int base, int size) {
263-
264253
int integer = base;
265254
for (int i = 0; i < size; i++) {
266255
integer = (integer << 8) | (buffer.get() & 0xFF);
@@ -281,41 +270,28 @@ private FloatNode decodeFloat() {
281270
return new FloatNode(this.threadBuffer.get().getFloat());
282271
}
283272

284-
private Result decodeBoolean(int size, int offset) {
285-
BooleanNode b = size == 0 ? BooleanNode.FALSE : BooleanNode.TRUE;
286-
287-
return new Result(b, offset);
273+
private static BooleanNode decodeBoolean(int size) {
274+
return size == 0 ? BooleanNode.FALSE : BooleanNode.TRUE;
288275
}
289276

290277
private Result decodeArray(int size, int offset) throws IOException {
291-
if (this.DEBUG) {
292-
Log.debug("Array size", size);
293-
}
294-
295278
ArrayNode array = this.objectMapper.createArrayNode();
296279

297280
for (int i = 0; i < size; i++) {
298281
Result r = this.decode(offset);
299282
offset = r.getOffset();
300-
301-
if (this.DEBUG) {
302-
Log.debug("Value " + i, r.getNode().toString());
303-
}
304283
array.add(r.getNode());
305284
}
306285

307-
if (this.DEBUG) {
286+
if (Decoder.DEBUG) {
287+
Log.debug("Array size", size);
308288
Log.debug("Decoded array", array.toString());
309289
}
310290

311291
return new Result(array, offset);
312292
}
313293

314294
private Result decodeMap(int size, int offset) throws IOException {
315-
if (this.DEBUG) {
316-
Log.debug("Map size", size);
317-
}
318-
319295
ObjectNode map = this.objectMapper.createObjectNode();
320296

321297
for (int i = 0; i < size; i++) {
@@ -327,14 +303,11 @@ private Result decodeMap(int size, int offset) throws IOException {
327303
JsonNode value = valueResult.getNode();
328304
offset = valueResult.getOffset();
329305

330-
if (this.DEBUG) {
331-
Log.debug("Key " + i, key);
332-
Log.debug("Value " + i, value.toString());
333-
}
334306
map.put(key, value);
335307
}
336308

337-
if (this.DEBUG) {
309+
if (Decoder.DEBUG) {
310+
Log.debug("Map size", size);
338311
Log.debug("Decoded map", map.toString());
339312
}
340313

@@ -344,25 +317,19 @@ private Result decodeMap(int size, int offset) throws IOException {
344317

345318
private int[] sizeFromCtrlByte(int ctrlByte, int offset) {
346319
int size = ctrlByte & 0x1f;
347-
348-
if (size < 29) {
349-
return new int[] { size, offset };
350-
}
351-
352-
int bytesToRead = size - 28;
320+
int bytesToRead = size < 29 ? 0 : size - 28;
353321

354322
if (size == 29) {
355323
int i = this.decodeInteger(bytesToRead);
356324
size = 29 + i;
357325
} else if (size == 30) {
358326
int i = this.decodeInteger(bytesToRead);
359327
size = 285 + i;
360-
} else {
328+
} else if (size > 30) {
361329
int i = this.decodeInteger(bytesToRead)
362330
& (0x0FFFFFFF >>> (32 - (8 * bytesToRead)));
363331
size = 65821 + i;
364332
}
365-
366333
return new int[] { size, offset + bytesToRead };
367334
}
368335

‎src/main/java/com/maxmind/maxminddb/Log.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ static void debugNewLine() {
3838
}
3939

4040
static void debug(String message) {
41+
// XXX - eventually we should probably switch to a real logger.
4142
System.out.println(message);
4243
}
4344

‎src/main/java/com/maxmind/maxminddb/MaxMindDbReader.java‎

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,6 @@ public MaxMindDbReader(File database) throws IOException {
6666
*/
6767
public MaxMindDbReader(File database, FileMode fileMode) throws IOException {
6868
this.threadBuffer = new ThreadBuffer(database, fileMode);
69-
70-
/*
71-
* We need to make sure that whatever chunk we read will have the
72-
* metadata in it. The description metadata key is a hash of
73-
* descriptions, one per language. The description could be something
74-
* verbose like "GeoIP 2.0 City Database, Multilingual - English,
75-
* Chinese (Taiwan), Chinese (China), French, German, Portuguese" (but
76-
* with c. 20 languages). That comes out to about 250 bytes _per key_.
77-
* Multiply that by 20 languages, and the description alon ecould use up
78-
* about 5k. The other keys in the metadata are very, very tiny.
79-
*
80-
* Given all this, reading 20k seems fairly future-proof. We'd have to
81-
* have extremely long descriptions or descriptions in 80 languages
82-
* before this became too long.
83-
*/
8469
int start = this.findMetadataStart();
8570

8671
if (start < 0) {
@@ -91,9 +76,7 @@ public MaxMindDbReader(File database, FileMode fileMode) throws IOException {
9176
}
9277

9378
Decoder metadataDecoder = new Decoder(this.threadBuffer, 0);
94-
9579
this.metadata = new Metadata(metadataDecoder.decode(start).getNode());
96-
9780
this.decoder = new Decoder(this.threadBuffer,
9881
this.metadata.searchTreeSize + DATA_SECTION_SEPARATOR_SIZE);
9982

@@ -112,13 +95,10 @@ public MaxMindDbReader(File database, FileMode fileMode) throws IOException {
11295
* if a file I/O error occurs.
11396
*/
11497
public JsonNode get(InetAddress ipAddress) throws IOException {
115-
11698
int pointer = this.findAddressInTree(ipAddress);
117-
11899
if (pointer == 0) {
119100
return null;
120101
}
121-
122102
this.threadBuffer.get().position(pointer);
123103
return this.resolveDataPointer(pointer);
124104
}
@@ -154,7 +134,6 @@ int record = this.readNode(nodeNum, bit);
154134
Log.debug("Bit #", i);
155135
Log.debug("Bit value", bit);
156136
Log.debug("Record", bit == 1 ? "right" : "left");
157-
// Log.debug("Node count", this.metadata.nodeCount);
158137
Log.debug("Record value", record);
159138
}
160139

@@ -163,9 +142,7 @@ int record = this.readNode(nodeNum, bit);
163142
Log.debug("Record is empty");
164143
}
165144
return 0;
166-
}
167-
168-
if (record >= this.metadata.nodeCount) {
145+
} else if (record > this.metadata.nodeCount) {
169146
if (MaxMindDbReader.DEBUG) {
170147
Log.debug("Record is a data pointer");
171148
}
@@ -178,7 +155,6 @@ int record = this.readNode(nodeNum, bit);
178155

179156
nodeNum = record;
180157
}
181-
182158
throw new InvalidDatabaseException("Something bad happened");
183159
}
184160

@@ -217,11 +193,9 @@ private JsonNode resolveDataPointer(int pointer) throws IOException {
217193

218194
if (MaxMindDbReader.DEBUG) {
219195
int treeSize = this.metadata.searchTreeSize;
220-
221196
Log.debug("Resolved data pointer", "( " + pointer + " - "
222197
+ this.metadata.nodeCount + " ) + " + treeSize + " = "
223198
+ resolved);
224-
225199
}
226200

227201
// We only want the data from the decoder, not the offset where it was
@@ -230,7 +204,8 @@ private JsonNode resolveDataPointer(int pointer) throws IOException {
230204
}
231205

232206
/*
233-
* And here I though searching a file was a solved problem.
207+
* Apparently searching a file for a sequence is not a solved problem in
208+
* Java. This searches from the end of the file for metadata start.
234209
*
235210
* This is an extremely naive but reasonably readable implementation. There
236211
* are much faster algorithms (e.g., Boyer-Moore) for this if speed is ever

‎src/main/java/com/maxmind/maxminddb/ThreadBuffer.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
import com.maxmind.maxminddb.MaxMindDbReader.FileMode;
1212

1313
final class ThreadBuffer extends ThreadLocal<ByteBuffer> implements Closeable {
14-
// XXX - DO NOT PASS THIS OUTSIDE THIS CLASS.
14+
// DO NOT PASS THESE OUTSIDE THIS CLASS. Doing so will remove thread
15+
// safety.
1516
private final ByteBuffer buffer;
1617
private final RandomAccessFile raf;
1718
private final FileChannel fc;

0 commit comments

Comments
 (0)