Merge lp:~tjoneslo/akiban-server/add-rest-model-hash into lp:~akiban-technologies/akiban-server/trunk

[Thomas Jones-Low]

Per the Story, add a /model/hash REST API.

Returns a MD5 hash of the current space in the server as a Json object for user consumption.

Added a MessageDigestWriter (no one already written) to use the existing Space#JsonGenerate process for least amount of duplicated code.

Add test for new writer. Add simple test for REST API.

[Nathan Williams (nwilliams)]

Looks like something is slightly off with our computation. Perhaps it's the incremental digest updating (random guess)?

$ echo "This is a test string" |md5sum
b584c39f97dfe71ebceea3fdb860ed6c -

The digestResult variable and toString() behavior is a little funny. Seems like always printing the digest would be useful (toString being for debug, etc).

There is also a helper in SecurityService for formatting an md5 byte array. Perhaps slicing that out into util.Strings or such would be good.

review: Needs Fixing

[Thomas Jones-Low (tjoneslo)]

Updated the md5 generation (was building based on the UTF-16 java characters, now using UTF-8 as expected.

echo -n "This is a test string" |md5sum
c639efc1e98762233743a75e7798dd9c -

It now also matches the results produced by MySQL, Python, and two online MD5 hash generators.

Updated the MessageDigestWriter to use getFormatMD5 for the string value.

Refactor the formatMD5() into the Strings class. Also update the MD5 function to use the same.

review: Needs Resubmitting

[Nathan Williams (nwilliams)]

I think formatMD5 needs another parameter for upper vs lowercase to preserve the SercurityService needs. Just moving the old SecurityService implementation to Strings would probably suffice.

review: Needs Fixing

[Thomas Jones-Low (tjoneslo)]

Move the RestServiceFiles hash test to the rest.space test to get a consistent "space" to generate hash values for run to run.

Fix the SecurityService requirements for MD5 string to be in upper case in some cases.

review: Needs Resubmitting

[Nathan Williams (nwilliams)]

Sorry to be picky, and maybe this is premature optimization, but the new code is pretty wasteful. Construct a BigInteger, toString() on that, maybe construct another sting with leading zero, and append that into a StringBuilder, toString() on that, and then toUppper or toLower it. That's six lines and a lot of intermediate steps. The previous, simple loop was five and avoided it all.

Feel free to push back.

[Thomas Jones-Low (tjoneslo)]

Tired of re-writing the same function, do what any good programmer does. Find someone else's implementation and make a one line call to that. Plus test to verify the leading 0 is still there.

review: Needs Resubmitting

[Nathan Williams (nwilliams)]

Ha! Even better than my suggestion. Excellent.

review: Approve

=== modified file 'src/main/java/com/akiban/rest/resources/ModelResource.java'
--- src/main/java/com/akiban/rest/resources/ModelResource.java	2013-03-26 16:44:27 +0000
+++ src/main/java/com/akiban/rest/resources/ModelResource.java	2013-03-28 14:27:28 +0000
@@ -99,6 +99,30 @@
                 })
                 .build();
     }
+    
+    @GET
+    @Path("/hash" + OPTIONAL_SCHEMA)
+    @Produces(MEDIATYPE_JSON_JAVASCRIPT)
+    public Response hashOfSpace(@Context HttpServletRequest request,
+                                @PathParam("schema") String schemaParam) {
+        final String schema = getSchemaName(request, schemaParam);
+        checkSchemaAccessible(reqs.securityService, request, schema);
+        return RestResponseBuilder
+                .forRequest(request)
+                .body(new RestResponseBuilder.BodyGenerator() {
+                    @Override
+                    public void write(PrintWriter writer) throws Exception {
+                        try (Session session = reqs.sessionService.createSession();
+                             CloseableTransaction txn = reqs.transactionService.beginCloseableTransaction(session)) {
+                            Space space = spaceForAIS(session, schema);
+                            String json = space.toHash();
+                            writer.write(json);
+                            txn.commit();
+                        }
+                    }
+                })
+                .build();
+    }
 
     @POST
     @Path("/parse/{table}")

=== modified file 'src/main/java/com/akiban/server/entity/model/Space.java'
--- src/main/java/com/akiban/server/entity/model/Space.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/server/entity/model/Space.java	2013-03-28 14:27:28 +0000
@@ -17,6 +17,7 @@
 
 package com.akiban.server.entity.model;
 
+import com.akiban.util.MessageDigestWriter;
 import com.google.common.base.Function;
 import org.codehaus.jackson.JsonGenerator;
 import org.codehaus.jackson.map.JsonMappingException;
@@ -27,6 +28,8 @@
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringWriter;
+import java.io.Writer;
+import java.security.NoSuchAlgorithmException;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
@@ -102,19 +105,43 @@
     }
 
     public String toJson() {
+        StringWriter writer = new StringWriter(); 
+        jsonGenerate (writer);
+        return writer.toString();
+    }
+
+    public String toHash () {
         try {
             StringWriter writer = new StringWriter();
+            MessageDigestWriter md5writer = new MessageDigestWriter();
+            JsonGenerator generator = createJsonGenerator(writer);
+            generator.writeStartObject();
+            generator.writeFieldName("hash");
+            jsonGenerate (md5writer);
+            generator.writeString(md5writer.getFormatMD5());
+            generator.writeEndObject();
+            generator.flush();
+            writer.toString();
+            return writer.toString();
+        } catch (NoSuchAlgorithmException e) {
+            throw new RuntimeException(e);
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+    
+    private void jsonGenerate(Writer writer) {
+        try {
             JsonGenerator generator = createJsonGenerator(writer);
             generateJson(generator);
             generator.flush();
             writer.flush();
-            return writer.toString();
         }
         catch (IOException e) {
             throw new RuntimeException(e);
         }
     }
-
+    
     Space() {}
 
     private Map<String, Entity> entities = Collections.emptyMap();

=== modified file 'src/main/java/com/akiban/server/service/security/SecurityServiceImpl.java'
--- src/main/java/com/akiban/server/service/security/SecurityServiceImpl.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/server/service/security/SecurityServiceImpl.java	2013-03-28 14:27:28 +0000
@@ -31,7 +31,7 @@
 import com.akiban.server.store.SchemaManager;
 import com.akiban.sql.server.ServerCallContextStack;
 import com.akiban.sql.server.ServerQueryContext;
-import com.akiban.sql.server.ServerSession;
+import com.akiban.util.Strings;
 
 import com.google.inject.Inject;
 import org.slf4j.Logger;
@@ -52,9 +52,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Properties;
 
 public class SecurityServiceImpl implements SecurityService, Service {
@@ -259,7 +257,6 @@
     public void deleteUser(String name) {
         Connection conn = null;
         PreparedStatement stmt = null;
-        boolean success = false;
         try {
             conn = openConnection();
             stmt = conn.prepareStatement(DELETE_USER_USER_ROLES_SQL);
@@ -287,7 +284,6 @@
     public void changeUserPassword(String name, String password) {
         Connection conn = null;
         PreparedStatement stmt = null;
-        boolean success = false;
         try {
             conn = openConnection();
             stmt = conn.prepareStatement(CHANGE_USER_PASSWORD_SQL);
@@ -315,7 +311,6 @@
         User user = null;
         Connection conn = null;
         PreparedStatement stmt = null;
-        boolean success = false;
         try {
             conn = openConnection();
             stmt = conn.prepareStatement(GET_USER_SQL);
@@ -347,7 +342,6 @@
         User user = null;
         Connection conn = null;
         PreparedStatement stmt = null;
-        boolean success = false;
         try {
             conn = openConnection();
             stmt = conn.prepareStatement(GET_USER_SQL);
@@ -434,18 +428,11 @@
         }
     }
 
-    protected final char[] HEX_UC = "0123456789ABCDEF".toCharArray();
-    protected final char[] HEX_LC = "0123456789abcdef".toCharArray();
-
     protected String formatMD5(byte[] md5, boolean forDigest) {
         StringBuilder str = new StringBuilder();
         str.append(forDigest ? "MD5:" : "md5");
-        final char[] hex = forDigest ? HEX_UC : HEX_LC;
-        for (int i = 0; i < md5.length; i++) {
-            int b = md5[i] & 0xFF;
-            str.append(hex[b >> 4]);
-            str.append(hex[b & 0xF]);
-        }
+        // Strings#formatMD5 wants toLowerCase for second parameter, inverse of the forDigest flag
+        str.append(Strings.formatMD5(md5, !forDigest));
         return str.toString();
     }
 
@@ -453,7 +440,6 @@
     public void clearAll(Session session) {
         Connection conn = null;
         Statement stmt = null;
-        boolean success = false;
         try {
             conn = openConnection();
             stmt = conn.createStatement();
@@ -461,7 +447,6 @@
             stmt.execute("DELETE FROM users");
             stmt.execute("DELETE FROM roles");
             conn.commit();
-            success = true;
         }
         catch (SQLException ex) {
             throw new SecurityException("Error adding role", ex);

=== modified file 'src/main/java/com/akiban/server/types3/mcompat/mfuncs/MD5.java'
--- src/main/java/com/akiban/server/types3/mcompat/mfuncs/MD5.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/server/types3/mcompat/mfuncs/MD5.java	2013-03-28 14:27:28 +0000
@@ -26,6 +26,8 @@
 import com.akiban.server.types3.pvalue.PValueTarget;
 import com.akiban.server.types3.texpressions.TInputSetBuilder;
 import com.akiban.server.types3.texpressions.TScalarBase;
+import com.akiban.util.Strings;
+
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
@@ -48,22 +50,8 @@
         {
             MessageDigest md = MessageDigest.getInstance("MD5");
             byte ret[] = md.digest(inputs.get(0).getBytes());
-            StringBuilder retStr = new StringBuilder(32);
-            int lo, hi;
-
-            for (byte b : ret)
-            {
-                lo = b & 0x0f;
-                hi = (b & 0xf0) >>> 4;
-                
-                retStr.append((char)(hi > 9
-                                     ? 'a' + hi - 10
-                                     : '0' + hi));
-                retStr.append((char)(lo > 9
-                                     ? 'a' + lo - 10
-                                     : '0' + lo));
-            }
-            output.putString(retStr.toString(), null);
+            
+            output.putString(Strings.formatMD5(ret, true), null);
         }
         catch (NoSuchAlgorithmException ex)
         {

=== added file 'src/main/java/com/akiban/util/MessageDigestWriter.java'
--- src/main/java/com/akiban/util/MessageDigestWriter.java	1970-01-01 00:00:00 +0000
+++ src/main/java/com/akiban/util/MessageDigestWriter.java	2013-03-28 14:27:28 +0000
@@ -0,0 +1,52 @@
+/**
+ * Copyright (C) 2009-2013 Akiban Technologies, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package com.akiban.util;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+
+public class MessageDigestWriter extends Writer {
+
+    private final MessageDigest digest;
+    
+    public MessageDigestWriter () throws NoSuchAlgorithmException {
+        digest = MessageDigest.getInstance("MD5");
+        digest.reset();
+    }
+    @Override
+    public void write(char[] cbuf, int off, int len) throws IOException {
+        byte[] bytes = String.valueOf(cbuf).substring(off, (len+off)).getBytes("UTF-8");
+        digest.update(bytes, 0, bytes.length);
+    }
+
+    @Override
+    public void flush() throws IOException {
+    }
+
+    @Override
+    public void close() throws IOException {
+    }
+
+    public MessageDigest getDigest() { 
+        return digest;
+    }
+    public String getFormatMD5() {
+        return Strings.formatMD5(digest.digest(), true);
+    }
+}

=== modified file 'src/main/java/com/akiban/util/Strings.java'
--- src/main/java/com/akiban/util/Strings.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/util/Strings.java	2013-03-28 14:27:28 +0000
@@ -20,6 +20,8 @@
 import com.akiban.server.error.InvalidParameterValueException;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
+
+import org.apache.commons.codec.binary.Hex;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -420,4 +422,8 @@
             throw new RuntimeException(e);
         }
     }
+    
+    public static String formatMD5(byte[] md5, boolean toLowerCase) {
+        return new String(Hex.encodeHex(md5, toLowerCase));
+    }
 }

=== added file 'src/test/java/com/akiban/util/MessageDigestWriterTest.java'
--- src/test/java/com/akiban/util/MessageDigestWriterTest.java	1970-01-01 00:00:00 +0000
+++ src/test/java/com/akiban/util/MessageDigestWriterTest.java	2013-03-28 14:27:28 +0000
@@ -0,0 +1,67 @@
+/**
+ * Copyright (C) 2009-2013 Akiban Technologies, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package com.akiban.util;
+
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+import java.security.NoSuchAlgorithmException;
+
+import org.junit.Test;
+
+public class MessageDigestWriterTest {
+
+    
+    @Test
+    public void testGetDigest() throws NoSuchAlgorithmException {
+        @SuppressWarnings("resource")
+        MessageDigestWriter writer = new MessageDigestWriter();
+        assertNotNull(writer.getDigest());
+    }
+
+    @Test
+    public void testToString() throws IOException, NoSuchAlgorithmException {
+        String test = "This is a test string";
+        MessageDigestWriter writer = new MessageDigestWriter();
+        writer.write(test);
+        writer.close();
+        assertEquals(writer.getFormatMD5(), "c639efc1e98762233743a75e7798dd9c");
+    }
+    
+    @Test
+    public void testoffset()  throws IOException, NoSuchAlgorithmException {
+        // Test my off-by-one problem is not present here
+        String test1 = "This is a test string";
+        String test2 =           "test string";
+
+        MessageDigestWriter writerA = new MessageDigestWriter();
+        
+        writerA.write(test2);
+        writerA.close();
+        String digest1 = writerA.getFormatMD5();
+        
+        MessageDigestWriter writerB = new MessageDigestWriter();
+        char[] chars = test1.toCharArray();
+        writerB.write(chars, 10, 11);
+        writerB.close();
+        String digest2 = writerB.getFormatMD5();
+        assertEquals(digest1, "6f8db599de986fab7a21625b7916589c");
+        assertEquals(digest1, digest2);
+               
+    }
+
+}

=== modified file 'src/test/java/com/akiban/util/StringsTest.java'
--- src/test/java/com/akiban/util/StringsTest.java	2013-03-22 20:05:57 +0000
+++ src/test/java/com/akiban/util/StringsTest.java	2013-03-28 14:27:28 +0000
@@ -45,4 +45,16 @@
 
         assertEquals("bytes", new WrappingByteSource(expected), actual);
     }
+    
+    @Test
+    public void formatMD5() {
+        byte[] md5 = {0x01, 0x39, (byte) 0xef, (byte) 0xc1, (byte) 0xe9, (byte) 0x86, 0x22, 0x33, 0x74, 0x3a, 0x75, 0x77, (byte) 0x98, (byte) 0xdd, (byte) 0x9c};
+        
+        String expected = "0139efc1e9862233743a757798dd9c";
+        
+        String actual = Strings.formatMD5(md5, true);
+        
+        assertEquals ("bytes", expected, actual);
+        
+    }
 }

=== added file 'src/test/resources/com/akiban/rest/space/hash-test.expected'
--- src/test/resources/com/akiban/rest/space/hash-test.expected	1970-01-01 00:00:00 +0000
+++ src/test/resources/com/akiban/rest/space/hash-test.expected	2013-03-28 14:27:28 +0000
@@ -0,0 +1,3 @@
+{
+	"hash": "92159a7225ef31a02dcead921284f737"
+}
\ No newline at end of file

=== added file 'src/test/resources/com/akiban/rest/space/hash-test.get'
--- src/test/resources/com/akiban/rest/space/hash-test.get	1970-01-01 00:00:00 +0000
+++ src/test/resources/com/akiban/rest/space/hash-test.get	2013-03-28 14:27:28 +0000
@@ -0,0 +1,1 @@
+/model/hash/test/
\ No newline at end of file