From 337fe5bd3d55c64464906e5ee1658f40b166fe16 Mon Sep 17 00:00:00 2001 From: Pontus Melke Date: Fri, 1 Jul 2016 07:55:03 +0200 Subject: [PATCH] Code review: change so that char -> String, char[] -> List --- .../neo4j/bolt/v1/messaging/Neo4jPack.java | 12 ++++++--- .../neo4j/bolt/v1/packstream/PackStream.java | 5 ++++ .../bolt/v1/messaging/Neo4jPackTest.java | 26 +++++++++++-------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java index 04c84f7765a91..ecefa72ca27df 100644 --- a/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java +++ b/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/Neo4jPack.java @@ -66,6 +66,7 @@ public Packer( PackOutput output ) super( output ); } + @SuppressWarnings( "unchecked" ) public void pack( Object obj ) throws IOException { // Note: below uses instanceof for quick implementation, this should be swapped over @@ -96,8 +97,7 @@ else if ( obj instanceof String ) } else if (obj instanceof Character ) { - Character character = (Character) obj; - pack( character.toString() ); + pack( (Character) obj ); } else if ( obj instanceof Map ) { @@ -127,8 +127,12 @@ else if ( obj instanceof byte[] ) } else if ( obj instanceof char[] ) { - // Treat it as a String - pack( new String( (char[]) obj ) ); + char[] array = (char[]) obj; + packListHeader( array.length ); + for ( char item : array ) + { + pack( item ); + } } else if ( obj instanceof short[] ) { diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java index be2e9019b3ec1..0f388a4d57afa 100644 --- a/community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java +++ b/community/bolt/src/main/java/org/neo4j/bolt/v1/packstream/PackStream.java @@ -221,6 +221,11 @@ public void pack( char character ) throws IOException pack( new String( new char[]{character} ) ); } + public void pack( Character character ) throws IOException + { + pack( new String( new char[]{character} ) ); + } + public void pack( String value ) throws IOException { if ( value == null ) { packNull(); } diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/Neo4jPackTest.java b/community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/Neo4jPackTest.java index 3f860fb5af61b..09fd3aaf4b1ee 100644 --- a/community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/Neo4jPackTest.java +++ b/community/bolt/src/test/java/org/neo4j/bolt/v1/messaging/Neo4jPackTest.java @@ -36,6 +36,7 @@ import org.neo4j.graphdb.Path; import org.neo4j.kernel.impl.util.HexPrinter; +import static java.util.Arrays.asList; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; @@ -68,6 +69,7 @@ private Object unpacked( byte[] bytes ) throws IOException return unpacker.unpack(); } + @SuppressWarnings( "unchecked" ) @Test public void shouldBeAbleToPackAndUnpackListStream() throws IOException { @@ -91,6 +93,7 @@ public void shouldBeAbleToPackAndUnpackListStream() throws IOException assertThat( unpackedList, equalTo( expected ) ); } + @SuppressWarnings( "unchecked" ) @Test public void shouldBeAbleToPackAndUnpackMapStream() throws IOException { @@ -98,7 +101,7 @@ public void shouldBeAbleToPackAndUnpackMapStream() throws IOException PackedOutputArray output = new PackedOutputArray(); Neo4jPack.Packer packer = new Neo4jPack.Packer( output ); packer.packMapStreamHeader(); - for ( Map.Entry entry : ALICE.getAllProperties().entrySet() ) + for ( Map.Entry entry : ALICE.getAllProperties().entrySet() ) { packer.pack( entry.getKey() ); packer.pack( entry.getValue() ); @@ -108,17 +111,18 @@ public void shouldBeAbleToPackAndUnpackMapStream() throws IOException // Then assertThat( unpacked, instanceOf( Map.class ) ); - Map unpackedMap = (Map) unpacked; + Map unpackedMap = (Map) unpacked; assertThat( unpackedMap, equalTo( ALICE.getAllProperties() ) ); } + @SuppressWarnings( "unchecked" ) @Test public void shouldFailNicelyWhenPackingAMapWithUnpackableValues() throws IOException { // Given PackedOutputArray output = new PackedOutputArray(); Neo4jPack.Packer packer = new Neo4jPack.Packer( output ); - packer.packRawMap( map("unpackable", new Unpackable() ) ); + packer.packRawMap( map( "unpackable", new Unpackable() ) ); Object unpacked = unpacked( output.bytes() ); // Then @@ -151,10 +155,10 @@ public void shouldErrorOnUnpackingMapWithDuplicateKeys() throws IOException public void shouldHandleDeletedNodesGracefully() throws IOException { // Given - Node node = mock(Node.class); - when(node.getId()).thenReturn( 42L ); - doThrow( NotFoundException.class ).when(node).getAllProperties( ); - doThrow( NotFoundException.class ).when(node).getLabels( ); + Node node = mock( Node.class ); + when( node.getId() ).thenReturn( 42L ); + doThrow( NotFoundException.class ).when( node ).getAllProperties(); + doThrow( NotFoundException.class ).when( node ).getLabels(); // When byte[] packed = packed( node ); @@ -166,7 +170,7 @@ public void shouldHandleDeletedNodesGracefully() throws IOException // labels: [] (90) // props: {} (A0) //} - assertThat(HexPrinter.hex( packed ), equalTo("B3 4E 2A 90 A0")); + assertThat( HexPrinter.hex( packed ), equalTo( "B3 4E 2A 90 A0" ) ); } @Test @@ -214,7 +218,7 @@ public void shouldTreatSingleCharAsSingleCharacterString() throws IOException } @Test - public void shouldTreatCharArrayAsString() throws IOException + public void shouldTreatCharArrayAsListOfStrings() throws IOException { // Given PackedOutputArray output = new PackedOutputArray(); @@ -223,8 +227,8 @@ public void shouldTreatCharArrayAsString() throws IOException Object unpacked = unpacked( output.bytes() ); // Then - assertThat( unpacked, instanceOf( String.class ) ); - assertThat( unpacked, equalTo( "WHY" ) ); + assertThat( unpacked, instanceOf( List.class ) ); + assertThat( unpacked, equalTo( asList( "W", "H", "Y" ) ) ); } private static class Unpackable