{"thread":{"id":"17825","subject":"[JGIT PATCH] 1/2: Externalizable items","startedAt":"2009-02-16T16:45:55Z","lastAt":"2009-02-16T18:16:59Z","messageCount":6,"participants":["Nigel Magnay","Johannes Schindelin","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"104972","messageId":"320075ff0902160845m264f78cdh8dc5307b24f4c3ed@mail.gmail.com","threadId":"17825","inReplyTo":null,"subject":"[JGIT PATCH] 1/2: Externalizable items","fromName":"Nigel Magnay","fromEmail":"nigel.magnay@gmail.com","sentAt":"2009-02-16T16:45:55Z","receivedAt":"2009-02-16T16:45:55Z","isPatch":true,"sender":{"key":"nigel.magnay@gmail.com","avatar":"https://gravatar.com/avatar/d85cf38287bef3a8e4fa02358d2756d7589f8676c5eeb881ce2f6d731e4526c3?d=mp&s=160"},"body":"Make parts of jgit externalizable, so that they can be marshalled over\nthe wire or onto disk,\nusing formats from git mailing list.\n\nSigned-off-by: Nigel Magnay <nigel.magnay@gmail.com>\n---\n .../src/org/spearce/jgit/lib/ObjectId.java         |   31 ++++-\n .../src/org/spearce/jgit/transport/RefSpec.java    |   77 +++++++----\n .../org/spearce/jgit/transport/RemoteConfig.java   |  142 +++++++++++++++++++-\n .../src/org/spearce/jgit/transport/URIish.java     |   31 ++++-\n 4 files changed, 252 insertions(+), 29 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\nb/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\nindex 52ce0d4..1385325 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\n@@ -38,6 +38,10 @@\n\n package org.spearce.jgit.lib;\n\n+import java.io.Externalizable;\n+import java.io.IOException;\n+import java.io.ObjectInput;\n+import java.io.ObjectOutput;\n import java.io.UnsupportedEncodingException;\n\n import org.spearce.jgit.util.NB;\n@@ -45,7 +49,7 @@\n /**\n  * A SHA-1 abstraction.\n  */\n-public class ObjectId extends AnyObjectId {\n+public class ObjectId extends AnyObjectId implements Externalizable {\n \tprivate static final ObjectId ZEROID;\n\n \tprivate static final String ZEROID_STR;\n@@ -56,6 +60,13 @@\n \t}\n\n \t/**\n+\t * Empty constructor, for Externalizable.\n+\t */\n+\tpublic ObjectId() {\n+\t\t// For Externalizable\n+\t}\n+\t\n+\t/**\n \t * Get the special all-null ObjectId.\n \t *\n \t * @return the all-null ObjectId, often used to stand-in for no object.\n@@ -269,4 +280,22 @@ protected ObjectId(final AnyObjectId src) {\n \tpublic ObjectId toObjectId() {\n \t\treturn this;\n \t}\n+\n+\tpublic void readExternal(ObjectInput in) throws IOException,\n+\t\t\tClassNotFoundException {\n+\t\tbyte[] sha1 = new byte[20];\n+\t\tin.read(sha1);\n+\t\t\n+\t\tw1 = NB.decodeInt32(sha1, 0);\n+\t\tw2 = NB.decodeInt32(sha1, 4);\n+\t\tw3 = NB.decodeInt32(sha1, 8);\n+\t\tw4 = NB.decodeInt32(sha1, 12);\n+\t\tw5 = NB.decodeInt32(sha1, 16);\n+\t}\n+\n+\tpublic void writeExternal(ObjectOutput out) throws IOException {\n+\t\tbyte[] sha1 = new byte[20];\n+\t\tcopyRawTo(sha1, 0);\n+\t\tout.write(sha1);\n+\t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RefSpec.java\nb/org.spearce.jgit/src/org/spearce/jgit/transport/RefSpec.java\nindex 521110b..0ee89b0 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/RefSpec.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RefSpec.java\n@@ -37,6 +37,11 @@\n\n package org.spearce.jgit.transport;\n\n+import java.io.Externalizable;\n+import java.io.IOException;\n+import java.io.ObjectInput;\n+import java.io.ObjectOutput;\n+\n import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.Ref;\n\n@@ -46,7 +51,7 @@\n  * A ref specification provides matching support and limited rules to rewrite a\n  * reference in one repository to another reference in another repository.\n  */\n-public class RefSpec {\n+public class RefSpec implements Externalizable {\n \t/**\n \t * Suffix for wildcard ref spec component, that indicate matching all refs\n \t * with specified prefix.\n@@ -109,30 +114,7 @@ public RefSpec() {\n \t *             the specification is invalid.\n \t */\n \tpublic RefSpec(final String spec) {\n-\t\tString s = spec;\n-\t\tif (s.startsWith(\"+\")) {\n-\t\t\tforce = true;\n-\t\t\ts = s.substring(1);\n-\t\t}\n-\n-\t\tfinal int c = s.indexOf(':');\n-\t\tif (c == 0) {\n-\t\t\ts = s.substring(1);\n-\t\t\tif (isWildcard(s))\n-\t\t\t\tthrow new IllegalArgumentException(\"Invalid wildcards \" + spec);\n-\t\t\tdstName = s;\n-\t\t} else if (c > 0) {\n-\t\t\tsrcName = s.substring(0, c);\n-\t\t\tdstName = s.substring(c + 1);\n-\t\t\tif (isWildcard(srcName) && isWildcard(dstName))\n-\t\t\t\twildcard = true;\n-\t\t\telse if (isWildcard(srcName) || isWildcard(dstName))\n-\t\t\t\tthrow new IllegalArgumentException(\"Invalid wildcards \" + spec);\n-\t\t} else {\n-\t\t\tif (isWildcard(s))\n-\t\t\t\tthrow new IllegalArgumentException(\"Invalid wildcards \" + spec);\n-\t\t\tsrcName = s;\n-\t\t}\n+\t\tinitializeFromString(spec);\n \t}\n\n \t/**\n@@ -161,6 +143,42 @@ private RefSpec(final RefSpec p) {\n \t}\n\n \t/**\n+\t * Initialize the ref specification from a string.\n+\t *\n+\t * @param spec\n+\t *            string describing the specification.\n+\t * @throws IllegalArgumentException\n+\t *             the specification is invalid.\n+\t */\n+\tprivate void initializeFromString(final String spec) {\n+\t\tsrcName = null;\n+\t\tString s = spec;\n+\t\tif (s.startsWith(\"+\")) {\n+\t\t\tforce = true;\n+\t\t\ts = s.substring(1);\n+\t\t}\n+\n+\t\tfinal int c = s.indexOf(':');\n+\t\tif (c == 0) {\n+\t\t\ts = s.substring(1);\n+\t\t\tif (isWildcard(s))\n+\t\t\t\tthrow new IllegalArgumentException(\"Invalid wildcards \" + spec);\n+\t\t\tdstName = s;\n+\t\t} else if (c > 0) {\n+\t\t\tsrcName = s.substring(0, c);\n+\t\t\tdstName = s.substring(c + 1);\n+\t\t\tif (isWildcard(srcName) && isWildcard(dstName))\n+\t\t\t\twildcard = true;\n+\t\t\telse if (isWildcard(srcName) || isWildcard(dstName))\n+\t\t\t\tthrow new IllegalArgumentException(\"Invalid wildcards \" + spec);\n+\t\t} else {\n+\t\t\tif (isWildcard(s))\n+\t\t\t\tthrow new IllegalArgumentException(\"Invalid wildcards \" + spec);\n+\t\t\tsrcName = s;\n+\t\t}\n+\t}\n+\t\n+\t/**\n \t * Check if this specification wants to forcefully update the destination.\n \t *\n \t * @return true if this specification asks for updates without merge tests.\n@@ -421,4 +439,13 @@ public String toString() {\n \t\t}\n \t\treturn r.toString();\n \t}\n+\n+\tpublic void readExternal(ObjectInput in) throws IOException,\n+\t\t\tClassNotFoundException {\n+\t\tinitializeFromString(in.readUTF());\t\n+\t}\n+\n+\tpublic void writeExternal(ObjectOutput out) throws IOException {\n+\t\tout.writeUTF(toString());\n+\t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\nb/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\nindex 5bbf664..22443b4 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n@@ -38,10 +38,17 @@\n\n package org.spearce.jgit.transport;\n\n+import java.io.Externalizable;\n+import java.io.IOException;\n+import java.io.ObjectInput;\n+import java.io.ObjectOutput;\n import java.net.URISyntaxException;\n import java.util.ArrayList;\n+import java.util.Collection;\n import java.util.Collections;\n+import java.util.HashMap;\n import java.util.List;\n+import java.util.Map;\n\n import org.spearce.jgit.lib.RepositoryConfig;\n\n@@ -53,7 +60,7 @@\n  * describing how refs should be transferred between this repository and the\n  * remote repository.\n  */\n-public class RemoteConfig {\n+public class RemoteConfig implements Externalizable {\n \tprivate static final String SECTION = \"remote\";\n\n \tprivate static final String KEY_URL = \"url\";\n@@ -166,6 +173,17 @@ public RemoteConfig(final RepositoryConfig rc,\nfinal String remoteName)\n \t}\n\n \t/**\n+\t * Construct an empty remote config.\n+\t */\n+\tpublic RemoteConfig() {\n+\t\turis = new ArrayList<URIish>();\n+\t\tfetch = new ArrayList<RefSpec>();\n+\t\tpush = new ArrayList<RefSpec>();\n+\t\tuploadpack = DEFAULT_UPLOAD_PACK;\n+\t\treceivepack = DEFAULT_RECEIVE_PACK;\n+\t}\n+\t\n+\t/**\n \t * Update this remote's definition within the configuration.\n \t *\n \t * @param rc\n@@ -382,4 +400,126 @@ public TagOpt getTagOpt() {\n \tpublic void setTagOpt(final TagOpt option) {\n \t\ttagopt = option != null ? option : TagOpt.AUTO_FOLLOW;\n \t}\n+\n+\tprivate Map<String, Collection<String>> toMap() {\n+\t\tMap<String, Collection<String>> map = new HashMap<String,\nCollection<String>>();\n+\t\t\n+\t\tif (uris.size() > 0) {\n+\t\t\tCollection<String> values = new ArrayList<String>();\n+\t\t\tfor (URIish uri : uris) {\n+\t\t\t\tvalues.add(uri.toPrivateString());\n+\t\t\t}\n+\t\t\tmap.put(KEY_URL, values);\n+\t\t}\n+\t\t\n+\t\tif (fetch.size() > 0) {\n+\t\t\tCollection<String> values = new ArrayList<String>();\n+\t\t\tfor (RefSpec refspec : fetch) {\n+\t\t\t\tvalues.add(refspec.toString());\n+\t\t\t}\n+\t\t\tmap.put(KEY_FETCH, values);\n+\t\t}\n+\t\t\n+\t\tif (push.size() > 0) {\n+\t\t\tCollection<String> values = new ArrayList<String>();\n+\t\t\tfor (RefSpec refspec : push) {\n+\t\t\t\tvalues.add(refspec.toString());\n+\t\t\t}\n+\t\t\tmap.put(KEY_PUSH, values);\n+\t\t}\n+\t\t\n+\t\tCollection<String> uploads = new ArrayList<String>(1);\n+\t\tuploads.add(uploadpack);\n+\t\tmap.put(KEY_UPLOADPACK, uploads);\n+\t\t\n+\t\tCollection<String> receives = new ArrayList<String>(1);\n+\t\treceives.add(uploadpack);\n+\t\tmap.put(KEY_RECEIVEPACK, receives);\n+\t\t\n+\t\tCollection<String> tag = new ArrayList<String>(1);\n+\t\ttag.add(tagopt.option());\n+\t\tmap.put(KEY_TAGOPT, tag);\n+\t\t\n+\t\t\n+\t\treturn map;\n+\t}\n+\n+\tprivate void fromMap(Map<String, Collection<String>> map)\n+\t\t\tthrows URISyntaxException {\n+\t\tfor (Map.Entry<String, Collection<String>> entry : map.entrySet()) {\n+\t\t\tString key = entry.getKey();\n+\n+\t\t\tif (key.equals(KEY_URL)) {\n+\t\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\t\turis.add(new URIish(value));\n+\t\t\t\t}\n+\t\t\t} else if (key.equals(KEY_FETCH)) {\n+\t\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\t\tfetch.add(new RefSpec(value));\n+\t\t\t\t}\n+\t\t\t} else if (key.equals(KEY_PUSH)) {\n+\t\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\t\tpush.add(new RefSpec(value));\n+\t\t\t\t}\n+\t\t\t} else if (key.equals(KEY_UPLOADPACK)) {\n+\t\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\t\tuploadpack = value;\n+\t\t\t\t}\n+\t\t\t} else if (key.equals(KEY_RECEIVEPACK)) {\n+\t\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\t\treceivepack = value;\n+\t\t\t\t}\n+\t\t\t} else if (key.equals(KEY_TAGOPT)) {\n+\t\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\t\ttagopt = TagOpt.fromOption(value);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tpublic void readExternal(ObjectInput in) throws IOException,\n+\t\t\tClassNotFoundException {\n+\t\tname = in.readUTF();\n+\t\tint items = in.readInt();\n+\n+\t\tMap<String, Collection<String>> map = new HashMap<String,\nCollection<String>>();\n+\t\tfor (int i = 0; i < items; i++) {\n+\t\t\tString key = in.readUTF();\n+\t\t\tString value = in.readUTF();\n+\n+\t\t\tCollection<String> values = map.get(key);\n+\t\t\tif (values == null) {\n+\t\t\t\tvalues = new ArrayList<String>();\n+\t\t\t\tmap.put(key, values);\n+\t\t\t}\n+\n+\t\t\tvalues.add(value);\n+\t\t}\n+\n+\t\ttry {\n+\t\t\tfromMap(map);\n+\t\t} catch (URISyntaxException ex) {\n+\t\t\tthrow new IOException(\"Problem reading RemoteConfig map\");\n+\t\t}\n+\t}\n+\n+\tpublic void writeExternal(ObjectOutput out) throws IOException {\n+\t\tMap<String, Collection<String>> map = toMap();\n+\t\t\n+\t\tint size = 0;\n+\t\t\n+\t\tfor (Map.Entry<String, Collection<String>> entry : map.entrySet()) {\n+\t\t\tsize += entry.getValue().size();\n+\t\t}\n+\t\t\n+\t\tout.writeUTF(name);\n+\t\tout.writeInt(size);\n+\n+\t\tfor (Map.Entry<String, Collection<String>> entry : map.entrySet()) {\n+\t\t\tfor (String value : entry.getValue()) {\n+\t\t\t\tout.writeUTF(entry.getKey());\n+\t\t\t\tout.writeUTF(value);\n+\t\t\t}\n+\t\t}\n+\t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/URIish.java\nb/org.spearce.jgit/src/org/spearce/jgit/transport/URIish.java\nindex b86e00c..05e24b2 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/URIish.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/URIish.java\n@@ -38,6 +38,10 @@\n\n package org.spearce.jgit.transport;\n\n+import java.io.Externalizable;\n+import java.io.IOException;\n+import java.io.ObjectInput;\n+import java.io.ObjectOutput;\n import java.net.URISyntaxException;\n import java.net.URL;\n import java.util.regex.Matcher;\n@@ -49,7 +53,7 @@\n  * RFC 2396 URI's is that no URI encoding/decoding ever takes place. A space or\n  * any special character is written as-is.\n  */\n-public class URIish {\n+public class URIish implements Externalizable {\n \tprivate static final Pattern FULL_URI = Pattern\n \t\t\t.compile(\"^(?:([a-z][a-z0-9+-]+)://(?:([^/]+?)(?::([^/]+?))?@)?(?:([^/]+?))?(?::(\\\\d+))?)?((?:[A-Za-z]:)?/.+)$\");\n\n@@ -75,7 +79,17 @@\n \t * @throws URISyntaxException\n \t */\n \tpublic URIish(String s) throws URISyntaxException {\n-\t\ts = s.replace('\\\\', '/');\n+\t\tinitializeFromString(s);\n+\t}\n+\t\n+\t/**\n+\t * Set fields from string based URI.\n+\t *\n+\t * @param s\n+\t * @throws URISyntaxException\n+\t */\n+\tprivate void initializeFromString(String s)  throws URISyntaxException {\n+\t    s = s.replace('\\\\', '/');\n \t\tMatcher matcher = FULL_URI.matcher(s);\n \t\tif (matcher.matches()) {\n \t\t\tscheme = matcher.group(1);\n@@ -357,4 +371,17 @@ private String format(final boolean includePassword) {\n\n \t\treturn r.toString();\n \t}\n+\n+\tpublic void readExternal(ObjectInput in) throws IOException,\n+\t\t\tClassNotFoundException {\n+\t    try {\n+\t\t\tinitializeFromString(in.readUTF());\n+\t\t} catch (URISyntaxException e) {\n+\t\t\tthrow new IOException(\"Incorrect format URI\");\n+\t\t}\n+\t}\n+\n+\tpublic void writeExternal(ObjectOutput out) throws IOException {\n+\t\tout.writeUTF(format(true));\n+\t}\n }\n-- \n1.6.0.2\n"},{"id":"104975","messageId":"alpine.DEB.1.00.0902161758030.6289@intel-tinevez-2-302","threadId":"17825","inReplyTo":"320075ff0902160845m264f78cdh8dc5307b24f4c3ed@mail.gmail.com","subject":"Re: [JGIT PATCH] 1/2: Externalizable items","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-16T16:59:23Z","receivedAt":"2009-02-16T16:59:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Feb 2009, Nigel Magnay wrote:\n\n> Make parts of jgit externalizable, so that they can be marshalled over\n> the wire or onto disk,\n> using formats from git mailing list.\n\nHmm.  I have to be honest and admit that I have no idea what you mean by \nexternalizable.  From the \"marshalling\" comment, I'd have assumed that you \nmean Serializable, but then I still do not understand what \"formats from \ngit mailing list\" are.  And that being a regular of said list for quite \nsome time...\n\nCare to enlighten me?\n\nCiao,\nDscho\n"},{"id":"104977","messageId":"320075ff0902160910v723d077bp5eb3559c1d8f998a@mail.gmail.com","threadId":"17825","inReplyTo":"alpine.DEB.1.00.0902161758030.6289@intel-tinevez-2-302","subject":"Re: [JGIT PATCH] 1/2: Externalizable items","fromName":"Nigel Magnay","fromEmail":"nigel.magnay@gmail.com","sentAt":"2009-02-16T17:10:03Z","receivedAt":"2009-02-16T17:10:03Z","isPatch":true,"sender":{"key":"nigel.magnay@gmail.com","avatar":"https://gravatar.com/avatar/d85cf38287bef3a8e4fa02358d2756d7589f8676c5eeb881ce2f6d731e4526c3?d=mp&s=160"},"body":"Externalizable is the java standard for Serializable, but controlling\nthe exact format (rather than using introspection).\n\nhttp://n2.nabble.com/-PATCH-JGIT--Minor-%3A-Make-ObjectId%2C-RemoteConfig-Serializable-td2286559.html#none\n\n\nOn Mon, Feb 16, 2009 at 4:59 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Mon, 16 Feb 2009, Nigel Magnay wrote:\n>\n>> Make parts of jgit externalizable, so that they can be marshalled over\n>> the wire or onto disk,\n>> using formats from git mailing list.\n>\n> Hmm.  I have to be honest and admit that I have no idea what you mean by\n> externalizable.  From the \"marshalling\" comment, I'd have assumed that you\n> mean Serializable, but then I still do not understand what \"formats from\n> git mailing list\" are.  And that being a regular of said list for quite\n> some time...\n>\n> Care to enlighten me?\n>\n> Ciao,\n> Dscho\n>\n"},{"id":"104979","messageId":"20090216172025.GE18525@spearce.org","threadId":"17825","inReplyTo":"320075ff0902160845m264f78cdh8dc5307b24f4c3ed@mail.gmail.com","subject":"Re: [JGIT PATCH] 1/2: Externalizable items","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-16T17:20:25Z","receivedAt":"2009-02-16T17:20:25Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nigel Magnay <nigel.magnay@gmail.com> wrote:\n> Make parts of jgit externalizable, so that they can be marshalled over\n> the wire or onto disk,\n> using formats from git mailing list.\n\nAs Dscho pointed out, a bit more detail here would be appreciated.\n \n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\n> b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\n> index 52ce0d4..1385325 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectId.java\n> @@ -56,6 +60,13 @@\n>  \t}\n> \n>  \t/**\n> +\t * Empty constructor, for Externalizable.\n> +\t */\n> +\tpublic ObjectId() {\n> +\t\t// For Externalizable\n> +\t}\n\nYikes.  Do we really need a public no-arg constructor for\nExternalizable?  If we do, maybe we should use Serializable instead\nso we can hide this constructor.  I don't like the idea of people\ncreating ObjectId.zeroId() by new ObjectId().  That's not a pattern\nwe should encourage.\n\n> @@ -269,4 +280,22 @@ protected ObjectId(final AnyObjectId src) {\n>  \tpublic ObjectId toObjectId() {\n>  \t\treturn this;\n>  \t}\n> +\n> +\tpublic void readExternal(ObjectInput in) throws IOException,\n> +\t\t\tClassNotFoundException {\n> +\t\tbyte[] sha1 = new byte[20];\n> +\t\tin.read(sha1);\n> +\t\t\n> +\t\tw1 = NB.decodeInt32(sha1, 0);\n> +\t\tw2 = NB.decodeInt32(sha1, 4);\n> +\t\tw3 = NB.decodeInt32(sha1, 8);\n> +\t\tw4 = NB.decodeInt32(sha1, 12);\n> +\t\tw5 = NB.decodeInt32(sha1, 16);\n> +\t}\n> +\n> +\tpublic void writeExternal(ObjectOutput out) throws IOException {\n> +\t\tbyte[] sha1 = new byte[20];\n> +\t\tcopyRawTo(sha1, 0);\n> +\t\tout.write(sha1);\n> +\t}\n\nHmm.  I was thinking of just writing the 5 ints out, and reading\nthe 5 ints back in.  We're always talking to another Java process.\nThe ints are written in network byte order anyway on a serialization\nstream.  Doing this conversion to a byte[] thrases the caller's\nper-thread new generation rather hard.  I think applications using\nthis type in a serialization stream would expect it to be quick.\n\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n> b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n> index 5bbf664..22443b4 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n> +\n> +\tpublic void readExternal(ObjectInput in) throws IOException,\n> +\t\t\tClassNotFoundException {\n> +\t\tname = in.readUTF();\n> +\t\tint items = in.readInt();\n> +\n> +\t\tMap<String, Collection<String>> map = new HashMap<String,\n> Collection<String>>();\n> +\t\tfor (int i = 0; i < items; i++) {\n> +\t\t\tString key = in.readUTF();\n> +\t\t\tString value = in.readUTF();\n\nWhy not just serialize the Map in the stream?\n\n-- \nShawn.\n"},{"id":"104987","messageId":"320075ff0902161009s1454e1feu5b3543f898112406@mail.gmail.com","threadId":"17825","inReplyTo":"20090216172025.GE18525@spearce.org","subject":"Re: [JGIT PATCH] 1/2: Externalizable items","fromName":"Nigel Magnay","fromEmail":"nigel.magnay@gmail.com","sentAt":"2009-02-16T18:09:01Z","receivedAt":"2009-02-16T18:09:01Z","isPatch":true,"sender":{"key":"nigel.magnay@gmail.com","avatar":"https://gravatar.com/avatar/d85cf38287bef3a8e4fa02358d2756d7589f8676c5eeb881ce2f6d731e4526c3?d=mp&s=160"},"body":"> Yikes.  Do we really need a public no-arg constructor for\n> Externalizable?  If we do, maybe we should use Serializable instead\n> so we can hide this constructor.  I don't like the idea of people\n> creating ObjectId.zeroId() by new ObjectId().  That's not a pattern\n> we should encourage.\n>\n\nYes, you have to have a public no-args constructor for Externalizable.\n\n I agree, it's hideous. But I thought that was known as you explicitly\nasked for Externalizable rather than Serializable with readObject /\nwriteObject... :-/\n\nMore than happy to re-roll with Serializable instead - do you want\nthis for all 4? (RemoteConfig also gained a no-args constructor\nbecause of Externalizable..)\n\n>> +     public void writeExternal(ObjectOutput out) throws IOException {\n>> +             byte[] sha1 = new byte[20];\n>> +             copyRawTo(sha1, 0);\n>> +             out.write(sha1);\n>> +     }\n>\n> Hmm.  I was thinking of just writing the 5 ints out, and reading\n> the 5 ints back in.  We're always talking to another Java process.\n> The ints are written in network byte order anyway on a serialization\n> stream.  Doing this conversion to a byte[] thrases the caller's\n> per-thread new generation rather hard.  I think applications using\n> this type in a serialization stream would expect it to be quick.\n\nI've taken the request for \"the 20 byte SHA-1\" too literally :-)\n\n> +             Map<String, Collection<String>> map = new HashMap<String,\n> Collection<String>>();\n> +             for (int i = 0; i < items; i++) {\n> +                     String key = in.readUTF();\n> +                     String value = in.readUTF();\n>Why not just serialize the Map in the stream?\n\nSure - if you're happy with that representation - it's not \" a map of\nkeys/values as it appears in the config \" though as it's a map to a\nlist because of the multi-values that are available for things like\nURL and Fetch.\n"},{"id":"104989","messageId":"20090216181659.GF18525@spearce.org","threadId":"17825","inReplyTo":"320075ff0902161009s1454e1feu5b3543f898112406@mail.gmail.com","subject":"Re: [JGIT PATCH] 1/2: Externalizable items","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-16T18:16:59Z","receivedAt":"2009-02-16T18:16:59Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nigel Magnay <nigel.magnay@gmail.com> wrote:\n> > Yikes.  Do we really need a public no-arg constructor for\n> > Externalizable?  If we do, maybe we should use Serializable instead\n> > so we can hide this constructor.  I don't like the idea of people\n> > creating ObjectId.zeroId() by new ObjectId().  That's not a pattern\n> > we should encourage.\n> >\n> \n> Yes, you have to have a public no-args constructor for Externalizable.\n> \n>  I agree, it's hideous. But I thought that was known as you explicitly\n> asked for Externalizable rather than Serializable with readObject /\n> writeObject... :-/\n\nI forgot/didn't remember that Externalizable has this hideous\nrequirement.  I'd rather not introduce a public no-arg constructor\njust for Java serialization.  So yea, if you needed to add a\nconstructor than we should instead use Serializable and define an\nexplicit serialVersionUID.  Sorry.\n \n> More than happy to re-roll with Serializable instead - do you want\n> this for all 4? (RemoteConfig also gained a no-args constructor\n> because of Externalizable..)\n\nYea, at least for ObjectId and RemoteConfig.\n \n> > +             Map<String, Collection<String>> map = new HashMap<String,\n> > Collection<String>>();\n> > +             for (int i = 0; i < items; i++) {\n> > +                     String key = in.readUTF();\n> > +                     String value = in.readUTF();\n> >Why not just serialize the Map in the stream?\n> \n> Sure - if you're happy with that representation - it's not \" a map of\n> keys/values as it appears in the config \" though as it's a map to a\n> list because of the multi-values that are available for things like\n> URL and Fetch.\n\nRight.\n\nThe format of HashMap and ArrayList is pretty stable on the wire,\nso we can depend on them not changing on us.  We might as well\njust serialize with those and save ourselves some code in the JGit\nlibrary.  I don't see a compelling reason here to use our own map\nformat on the wire.  Especially not if we are doing this conversion\nto a java.util.Map<String,java.util.List> just to dump to the wire.\n\nOn the other hand, you could modify the read and write code to do\ndirect writing of string pairs, and direct reading of string pairs,\nthat would save allocs in each direction and make RemoteConfig\nthat much easier to write out.  Of course to do that you can't use\na count header, but instead would want a sential marker to break\nthe reader out of its loop, like an empty key string.\n\n-- \nShawn.\n"}]}