Skip to content

Commit

Permalink
Fixes a -1 short -> int conversion issue w/ legacy index key id
Browse files Browse the repository at this point in the history
where we'd like to treat the short as unsigned, except for the value -1.
This commit ensure that -1 gets recognized properly.
  • Loading branch information
tinwelint committed Jul 28, 2015
1 parent 62b534c commit 273cdd1
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 9 deletions.
Expand Up @@ -30,7 +30,6 @@
import org.neo4j.kernel.impl.transaction.command.CommandRecordVisitor;
import org.neo4j.kernel.impl.transaction.command.NeoCommandHandler;

import static java.lang.Math.pow;
import static java.lang.String.format;

import static org.neo4j.collection.primitive.Primitive.intObjectMap;
Expand All @@ -47,6 +46,7 @@
*/
public class IndexDefineCommand extends Command
{
static final int HIGHEST_POSSIBLE_ID = 0xFFFF - 1; // -1 since the actual value -1 is reserved for all-ones
private final AtomicInteger nextIndexNameId = new AtomicInteger();
private final AtomicInteger nextKeyId = new AtomicInteger();
private Map<String,Integer> indexNameIdRange;
Expand Down Expand Up @@ -129,11 +129,11 @@ private int getOrAssignId( Map<String,Integer> stringToId, PrimitiveIntObjectMap
}

int nextIdInt = nextId.incrementAndGet();
if ( (nextIdInt & ~0xFFFF) != 0 )
if ( nextIdInt > HIGHEST_POSSIBLE_ID ) // >= since the actual value -1 is reserved for all-ones
{
throw new IllegalStateException( format(
"Modifying more than %d indexes in a single transaction is not supported",
(int)(pow( 2, 16 ) - 1) ) );
HIGHEST_POSSIBLE_ID + 1 ) );
}
id = nextIdInt;

Expand Down
Expand Up @@ -695,7 +695,8 @@ private Map<String,Integer> readMap( ReadableLogChannel channel ) throws IOExcep

private int getUnsignedShort( ReadableLogChannel channel ) throws IOException
{
return channel.getShort() & 0xFFFF;
int result = channel.getShort() & 0xFFFF;
return result == 0xFFFF ? -1 : result;
}

private IndexCommandHeader readIndexCommandHeader() throws ReadPastEndException, IOException
Expand Down
Expand Up @@ -30,20 +30,31 @@ public void testIndexCommandCreationEnforcesLimit() throws Exception
{
// Given
IndexDefineCommand idc = new IndexDefineCommand();
int max = (int) (Math.pow( 2, 16 ) - 1);
int count = IndexDefineCommand.HIGHEST_POSSIBLE_ID;

// When
for ( int i = 0; i < max; i++ )
for ( int i = 0; i < count; i++ )
{
idc.getOrAssignKeyId( "index" + i );
idc.getOrAssignKeyId( "key" + i );
idc.getOrAssignIndexNameId( "index" + i );
}

// Then
// it should break on the 64th
// it should break on too many
try
{
idc.getOrAssignKeyId( "dropThatOverflows" );
fail( "IndexDefineCommand should not allow more than 63 indexes per transaction" );
fail( "IndexDefineCommand should not allow more than " + count + " indexes per transaction" );
}
catch( IllegalStateException e )
{
// wonderful
}

try
{
idc.getOrAssignIndexNameId( "dropThatOverflows" );
fail( "IndexDefineCommand should not allow more than " + count + " keys per transaction" );
}
catch( IllegalStateException e )
{
Expand Down
@@ -0,0 +1,66 @@
/*
* Copyright (c) 2002-2015 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU 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 General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.transaction.command;

import org.junit.Test;

import org.neo4j.helpers.collection.MapUtil;
import org.neo4j.kernel.impl.index.IndexCommand;
import org.neo4j.kernel.impl.index.IndexCommand.RemoveCommand;
import org.neo4j.kernel.impl.index.IndexDefineCommand;
import org.neo4j.kernel.impl.index.IndexEntityType;
import org.neo4j.kernel.impl.transaction.log.CommandWriter;
import org.neo4j.kernel.impl.transaction.log.InMemoryLogChannel;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

public class PhysicalLogNeoCommandReaderV2_2_4Test
{
@Test
public void shouldReadNoKeyIdAsMinusOne() throws Exception
{
// GIVEN
InMemoryLogChannel channel = new InMemoryLogChannel();
CommandWriter writer = new CommandWriter( channel );
IndexDefineCommand definitions = new IndexDefineCommand();
int indexNameId = 10;
definitions.init(
MapUtil.<String,Integer>genericMap( "myindex", indexNameId ),
MapUtil.<String,Integer>genericMap() );
writer.visitIndexDefineCommand( definitions );
RemoveCommand removeCommand = new IndexCommand.RemoveCommand();
removeCommand.init( indexNameId, IndexEntityType.Node.id(), 1234, -1, null );
writer.visitIndexRemoveCommand( removeCommand );

// WHEN
PhysicalLogNeoCommandReaderV2_2_4 reader = new PhysicalLogNeoCommandReaderV2_2_4();
assertTrue( reader.read( channel ) instanceof IndexDefineCommand );
RemoveCommand readRemoveCommand = (RemoveCommand) reader.read( channel );

// THEN
assertEquals( removeCommand.getIndexNameId(), readRemoveCommand.getIndexNameId() );
assertEquals( removeCommand.getEntityType(), readRemoveCommand.getEntityType() );
assertEquals( removeCommand.getEntityId(), readRemoveCommand.getEntityId() );
assertEquals( removeCommand.getKeyId(), readRemoveCommand.getKeyId() );
assertNull( removeCommand.getValue() );
}
}

0 comments on commit 273cdd1

Please sign in to comment.