- Iterate through eepgetted subscription file instead of loading the whole thing into memory

apps/addressbook/java/src/net/i2p/addressbook/AddressBook.java

@@ -23,11 +23,14 @@ package net.i2p.addressbook;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 
 import net.i2p.I2PAppContext;
 import net.i2p.util.EepGet;
+import net.i2p.util.SecureFile;
 
 /**
  * An address book for storing human readable names mapped to base64 i2p
@@ -39,11 +42,12 @@ import net.i2p.util.EepGet;
  */
 class AddressBook {
 
-    private String location;
+    private final String location;
-
+    /** either addresses or subFile will be non-null, but not both */
-    private Map<String, String> addresses;
+    private final Map<String, String> addresses;
-
+    private final File subFile;
     private boolean modified;
+    private static final boolean DEBUG = true;
 
     /**
      * Construct an AddressBook from the contents of the Map addresses.
@@ -54,6 +58,8 @@ class AddressBook {
      */
     public AddressBook(Map<String, String> addresses) {
         this.addresses = addresses;
+        this.subFile = null;
+        this.location = null;
     }
 
     /*
@@ -81,6 +87,7 @@ class AddressBook {
     }
 */
     static final long MAX_SUB_SIZE = 3 * 1024 * 1024l; //about 5,000 hosts
+
     /**
      * Construct an AddressBook from the Subscription subscription. If the
      * address book at subscription has not changed since the last time it was
@@ -89,33 +96,46 @@ class AddressBook {
      * 
      * Yes, the EepGet fetch() is done in this constructor.
      * 
+     * This stores the subscription in a temporary file and does not read the whole thing into memory.
+     * An AddressBook created with this constructor may not be modified or written using write().
+     * It may be a merge source (an parameter for another AddressBook's merge())
+     * but may not be a merge target (this.merge() will throw an exception).
+     * 
      * @param subscription
      *            A Subscription instance pointing at a remote address book.
      * @param proxyHost hostname of proxy
      * @param proxyPort port number of proxy
      */
     public AddressBook(Subscription subscription, String proxyHost, int proxyPort) {
-        File tmp = new File(I2PAppContext.getGlobalContext().getTempDir(), "addressbook.tmp");
+        Map<String, String> a = null;
+        File subf = null;
+        try {
+            File tmp = SecureFile.createTempFile("addressbook", null, I2PAppContext.getGlobalContext().getTempDir());
+            EepGet get = new EepGet(I2PAppContext.getGlobalContext(), true,
+                    proxyHost, proxyPort, 0, -1l, MAX_SUB_SIZE, tmp.getAbsolutePath(), null,
+                    subscription.getLocation(), true, subscription.getEtag(), subscription.getLastModified(), null);
+            if (get.fetch()) {
+                subscription.setEtag(get.getETag());
+                subscription.setLastModified(get.getLastModified());
+                subscription.setLastFetched(I2PAppContext.getGlobalContext().clock().now());
+                subf = tmp;
+            } else {
+                a = Collections.EMPTY_MAP;
+                tmp.delete();
+            }
+        } catch (IOException ioe) {
+            a = Collections.EMPTY_MAP;
+        }
+        this.addresses = a;
+        this.subFile = subf;
         this.location = subscription.getLocation();
-        EepGet get = new EepGet(I2PAppContext.getGlobalContext(), true,
-                proxyHost, proxyPort, 0, -1l, MAX_SUB_SIZE, tmp.getAbsolutePath(), null,
-                subscription.getLocation(), true, subscription.getEtag(), subscription.getLastModified(), null);
-        if (get.fetch()) {
-            subscription.setEtag(get.getETag());
-            subscription.setLastModified(get.getLastModified());
-            subscription.setLastFetched(I2PAppContext.getGlobalContext().clock().now());
-        }
-        try {            
-            this.addresses = ConfigParser.parse(tmp);
-        } catch (IOException exp) {
-            this.addresses = new HashMap();
-        }
-        tmp.delete();
     }
 
     /**
      * Construct an AddressBook from the contents of the file at file. If the
-     * file cannot be read, construct an empty AddressBook
+     * file cannot be read, construct an empty AddressBook.
+     * This reads the entire file into memory.
+     * The resulting map is modifiable and may be a merge target.
      * 
      * @param file
      *            A File pointing at a file with lines in the format
@@ -124,22 +144,38 @@ class AddressBook {
      */
     public AddressBook(File file) {
         this.location = file.toString();
+        Map<String, String> a;
         try {
-            this.addresses = ConfigParser.parse(file);
+            a = ConfigParser.parse(file);
         } catch (IOException exp) {
-            this.addresses = new HashMap();
+            a = new HashMap();
         }
+        this.addresses = a;
+        this.subFile = null;
     }
 
     /**
-     * Return a Map containing the addresses in the AddressBook.
+     * Return an iterator over the addresses in the AddressBook.
-     * 
+     * @since 0.8.6
-     * @return A Map containing the addresses in the AddressBook, where the key
-     *         is a human readable name, and the value is a base64 i2p
-     *         destination.
      */
-    public Map<String, String> getAddresses() {
+    public Iterator<Map.Entry<String, String>> iterator() {
-        return this.addresses;
+        if (this.subFile != null)
+            return new ConfigIterator(this.subFile);
+        return this.addresses.entrySet().iterator();
+    }
+
+    /**
+     * Delete the temp file or clear the map.
+     * @since 0.8.6
+     */
+    public void delete() {
+        if (this.subFile != null) {
+            this.subFile.delete();
+        } else if (this.addresses != null) {
+            try {
+                this.addresses.clear();
+            } catch (UnsupportedOperationException uoe) {}
+        }
     }
 
     /**
@@ -147,19 +183,22 @@ class AddressBook {
      * 
      * @return A String representing either an abstract path, or a url,
      *         depending on how the instance was constructed.
+     *         Will be null if created with the Map constructor.
      */
     public String getLocation() {
         return this.location;
     }
 
     /**
-     * Return a string representation of the contents of the AddressBook.
+     * Return a string representation of the origin of the AddressBook.
      * 
-     * @return A String representing the contents of the AddressBook.
+     * @return A String representing the origin of the AddressBook.
      */
     @Override
     public String toString() {
-        return this.addresses.toString();
+        if (this.location != null)
+            return "Book from " + this.location;
+        return "Map containing " + this.addresses.size() + " entries";
     }
 
     private static final int MIN_DEST_LENGTH = 516;
@@ -222,16 +261,21 @@ class AddressBook {
      * @param overwrite True to overwrite
      * @param log
      *            The log to write messages about new addresses or conflicts to.
+     *
+     * @throws IllegalStateException if this was created with the Subscription constructor.
      */
     public void merge(AddressBook other, boolean overwrite, Log log) {
-        for (Map.Entry<String, String> entry : other.addresses.entrySet()) {
+        if (this.addresses == null)
+            throw new IllegalStateException();
+        for (Iterator<Map.Entry<String, String>> iter = other.iterator(); iter.hasNext(); ) {
+            Map.Entry<String, String> entry = iter.next();
             String otherKey = entry.getKey();
             String otherValue = entry.getValue();
 
             if (isValidKey(otherKey) && isValidDest(otherValue)) {
                 if (this.addresses.containsKey(otherKey) && !overwrite) {
-                    if (!this.addresses.get(otherKey).equals(otherValue)
+                    if (DEBUG && log != null &&
-                            && log != null) {
+                        !this.addresses.get(otherKey).equals(otherValue)) {
                         log.append("Conflict for " + otherKey + " from "
                                 + other.location
                                 + ". Destination in remote address book is "
@@ -256,8 +300,12 @@ class AddressBook {
      * 
      * @param file
      *            The file to write the contents of this AddressBook too.
+     *
+     * @throws IllegalStateException if this was created with the Subscription constructor.
      */
     public void write(File file) {
+        if (this.addresses == null)
+            throw new IllegalStateException();
         if (this.modified) {
             try {
                 ConfigParser.write(this.addresses, file);
@@ -271,8 +319,17 @@ class AddressBook {
      * Write this AddressBook out to the file it was read from. Requires that
      * AddressBook was constructed from a file on the local filesystem. If the
      * file cannot be writen to, this method will silently fail.
+     *
+     * @throws IllegalStateException if this was not created with the File constructor.
      */
     public void write() {
+        if (this.location == null || this.location.startsWith("http://"))
+            throw new IllegalStateException();
         this.write(new File(this.location));
     }
+
+    @Override
+    protected void finalize() {
+        delete();
+    }
 }

apps/addressbook/java/src/net/i2p/addressbook/ConfigIterator.java

@@ -0,0 +1,144 @@
+/*
+ * Copyright (c) 2004 Ragnarok
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+package net.i2p.addressbook;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+
+/**
+ *  A class to iterate through a hosts.txt or config file without
+ *  reading the whole thing into memory.
+ *  Keys are always converted to lower case.
+ *
+ *  Callers should iterate all the way through or call close()
+ *  to ensure the underlying stream is closed.
+ *
+ *  @since 0.8.6
+ */
+class ConfigIterator implements Iterator<Map.Entry<String, String>> {
+
+    private BufferedReader input;
+    private ConfigEntry next;
+
+    /**
+     *  A dummy iterator in which hasNext() is always false.
+     */
+    public ConfigIterator() {}
+
+    /**
+     *  An iterator over the key/value pairs in the file.
+     */
+    public ConfigIterator(File file) {
+        try {
+            FileInputStream fileStream = new FileInputStream(file);
+            input = new BufferedReader(new InputStreamReader(fileStream));
+        } catch (IOException ioe) {}
+    }
+
+    public boolean hasNext() {
+        if (input == null)
+            return false;
+        if (next != null)
+            return true;
+        try {
+            String inputLine = input.readLine();
+            while (inputLine != null) {
+                inputLine = ConfigParser.stripComments(inputLine);
+                String[] splitLine = inputLine.split("=");
+                if (splitLine.length == 2) {
+                    next = new ConfigEntry(splitLine[0].trim().toLowerCase(), splitLine[1].trim());
+                    return true;
+                }
+                inputLine = input.readLine();
+            }
+        } catch (IOException ioe) {}
+        try { input.close(); } catch (IOException ioe) {}
+        input = null;
+        next = null;
+        return false;
+    }
+
+    public Map.Entry<String, String> next() {
+        if (!hasNext())
+            throw new NoSuchElementException();
+        Map.Entry<String, String> rv = next;
+        next = null;
+        return rv;
+    }
+
+    public void remove() {
+        throw new UnsupportedOperationException();
+    }
+
+    public void close() {
+        if (input != null) {
+            try { input.close(); } catch (IOException ioe) {}
+        }
+    }
+
+    @Override
+    protected void finalize() {
+        close();
+    }
+
+    /**
+     *  The object returned by the iterator.
+     */
+    private static class ConfigEntry implements Map.Entry<String, String> {
+        private final String key;
+        private final String value;
+
+        public ConfigEntry(String k, String v) {
+            key = k;
+            value = v;
+        }
+
+        public String getKey() {
+            return key;
+        }
+
+        public String getValue() {
+            return value;
+        }
+
+        public String setValue(String v) {
+            throw new UnsupportedOperationException();
+        }
+
+        public int hashCode() {
+            return key.hashCode() ^ value.hashCode();
+        }
+
+        public boolean equals(Object o) {
+            if (!(o instanceof Map.Entry))
+                return false;
+            Map.Entry e = (Map.Entry) o;
+            return key.equals(e.getKey()) && value.equals(e.getValue());
+        }
+    }
+}

apps/addressbook/java/src/net/i2p/addressbook/Daemon.java

@@ -45,6 +45,7 @@ public class Daemon {
     public static final String VERSION = "2.0.4";
     private static final Daemon _instance = new Daemon();
     private boolean _running;
+    private static final boolean DEBUG = true;
     
     /**
      * Update the router and published address books using remote data from the
@@ -103,8 +104,15 @@ public class Daemon {
         Iterator<AddressBook> iter = subscriptions.iterator();
         while (iter.hasNext()) {
             // yes, the EepGet fetch() is done in next()
+            long start = System.currentTimeMillis();
             AddressBook sub = iter.next();
-            for (Map.Entry<String, String> entry : sub.getAddresses().entrySet()) {
+            long end = System.currentTimeMillis();
+            if (DEBUG && log != null)
+                log.append("Fetch of " + sub.getLocation() + " took " + (end - start));
+            start = end;
+            int old = 0, nnew = 0, invalid = 0, conflict = 0;
+            for (Iterator<Map.Entry<String, String>> eIter = sub.iterator(); eIter.hasNext(); ) {
+                Map.Entry<String, String> entry = eIter.next();
                 String key = entry.getKey();
                 Destination oldDest = router.lookup(key);
                 try {
@@ -127,21 +135,40 @@ public class Daemon {
                                 if (!success)
                                     log.append("Save to published addressbook " + published.getAbsolutePath() + " failed for new key " + key);
                             }
+                            nnew++;
                         } else if (log != null) {
                             log.append("Bad hostname " + key + " from "
                                    + sub.getLocation());
+                            invalid++;
                         }        
-                    } else if (!oldDest.toBase64().equals(entry.getValue()) && log != null) {
+                    } else if (DEBUG && log != null) {
-                        log.append("Conflict for " + key + " from "
+                        if (!oldDest.toBase64().equals(entry.getValue())) {
-                                   + sub.getLocation()
+                            log.append("Conflict for " + key + " from "
-                                   + ". Destination in remote address book is "
+                                       + sub.getLocation()
-                                   + entry.getValue());
+                                       + ". Destination in remote address book is "
+                                       + entry.getValue());
+                            conflict++;
+                        } else {
+                            old++;
+                        }
+                    } else {
+                        old++;
                     }
                 } catch (DataFormatException dfe) {
                     if (log != null)
                         log.append("Invalid b64 for" + key + " From: " + sub.getLocation());
+                    invalid++;
                 }
             }
+            if (DEBUG && log != null) {
+                log.append("Merge of " + sub.getLocation() + " into " + router +
+                           " took " + (System.currentTimeMillis() - start) + " ms with " +
+                           nnew + " new, " +
+                           old + " old, " +
+                           invalid + " invalid, " +
+                           conflict + " conflicts");
+            }
+            sub.delete();
         }
     }