From 845edc7968cb3f1b036863e1e4a4b3638c0cd9c5 Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 11 Mar 2016 19:05:18 +0100 Subject: [PATCH] Make sure id limit is respected This is done by validating id of the record being updated in CommonAbstractStore#updateRecord(). Same validation logic is used in IdGenerator to guard against illegal and too high ids. --- .../impl/store/CommonAbstractStore.java | 9 +- .../format/BaseOneByteHeaderRecordFormat.java | 4 +- .../impl/store/format/BaseRecordFormat.java | 13 ++- .../format/lowlimit/DynamicRecordFormat.java | 10 +- .../lowlimit/LabelTokenRecordFormat.java | 8 +- .../format/lowlimit/MetaDataRecordFormat.java | 10 +- .../format/lowlimit/NodeRecordFormat.java | 8 +- .../format/lowlimit/NodeRecordFormatV2_0.java | 9 +- .../PropertyKeyTokenRecordFormat.java | 8 +- .../format/lowlimit/PropertyRecordFormat.java | 8 +- .../RelationshipGroupRecordFormat.java | 9 +- .../lowlimit/RelationshipRecordFormat.java | 9 +- .../RelationshipRecordFormatV2_0.java | 11 +-- .../RelationshipTypeTokenRecordFormat.java | 8 +- .../format/lowlimit/TokenRecordFormat.java | 4 +- .../kernel/impl/store/id/IdGeneratorImpl.java | 24 ++--- .../IdCapacityExceededException.java | 30 ++++++ .../impl/store/id/validation/IdValidator.java | 91 +++++++++++++++++++ .../id/validation/NegativeIdException.java | 30 ++++++ .../id/validation/ReservedIdException.java | 30 ++++++ .../internal/BatchInserterImpl.java | 11 +-- .../batchimport/store/BatchingIdSequence.java | 4 +- .../impl/core/JumpingIdGeneratorFactory.java | 8 +- .../impl/store/CommonAbstractStoreTest.java | 64 ++++++++++++- ...owLimitTest.java => LowLimitV3_0Test.java} | 2 +- .../impl/store/id/IdGeneratorImplTest.java | 69 ++++++++++++-- .../unsafe/batchinsert/BigBatchStoreIT.java | 6 +- .../highlimit/BaseHighLimitRecordFormat.java | 8 +- .../format/highlimit/DynamicRecordFormat.java | 9 +- .../highlimit/PropertyRecordFormat.java | 12 +-- 30 files changed, 368 insertions(+), 158 deletions(-) create mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdCapacityExceededException.java create mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdValidator.java create mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/NegativeIdException.java create mode 100644 community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/ReservedIdException.java rename community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/{LowLimitTest.java => LowLimitV3_0Test.java} (97%) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java index 493316d8c67be..86a093f8cc6e0 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/CommonAbstractStore.java @@ -37,8 +37,8 @@ import org.neo4j.kernel.impl.store.format.RecordFormat; import org.neo4j.kernel.impl.store.id.IdGenerator; import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; -import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; import org.neo4j.kernel.impl.store.id.IdType; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; import org.neo4j.kernel.impl.store.record.Record; import org.neo4j.kernel.impl.store.record.RecordLoad; @@ -120,11 +120,6 @@ public CommonAbstractStore( this.log = logProvider.getLog( getClass() ); } - protected static long longFromIntAndMod( long base, long modifier ) - { - return modifier == 0 && base == IdGeneratorImpl.INTEGER_MINUS_ONE ? -1 : base | modifier; - } - void initialise( boolean createIfNotExists ) { try @@ -1009,6 +1004,8 @@ public RECORD getRecord( long id, RECORD record, RecordLoad mode ) public void updateRecord( RECORD record ) { long id = record.getId(); + IdValidator.assertValidId( id, recordFormat.getMaxId() ); + long pageId = pageIdForRecord( id ); int offset = offsetForId( id ); try ( PageCursor cursor = storeFile.io( pageId, PF_SHARED_WRITE_LOCK ) ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseOneByteHeaderRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseOneByteHeaderRecordFormat.java index 1c175ea42c561..a69bd8787512f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseOneByteHeaderRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseOneByteHeaderRecordFormat.java @@ -37,9 +37,9 @@ public abstract class BaseOneByteHeaderRecordFormat recordSize, int recordHeaderSize, - int inUseBitMaskForFirstByte ) + int inUseBitMaskForFirstByte, int idBits ) { - super( recordSize, recordHeaderSize ); + super( recordSize, recordHeaderSize, idBits ); this.inUseBitMaskForFirstByte = inUseBitMaskForFirstByte; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormat.java index d653a7a049d39..e861ca8897c4f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/BaseRecordFormat.java @@ -25,8 +25,8 @@ import org.neo4j.io.pagecache.PagedFile; import org.neo4j.kernel.impl.store.IntStoreHeader; import org.neo4j.kernel.impl.store.StoreHeader; -import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; import org.neo4j.kernel.impl.store.id.IdSequence; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; import org.neo4j.kernel.impl.store.record.Record; @@ -50,11 +50,13 @@ public static Function fixedRecordSize( int recordSize ) private final Function recordSize; private final int recordHeaderSize; + private final long maxId; - protected BaseRecordFormat( Function recordSize, int recordHeaderSize ) + protected BaseRecordFormat( Function recordSize, int recordHeaderSize, int idBits ) { this.recordSize = recordSize; this.recordHeaderSize = recordHeaderSize; + this.maxId = (1L << idBits) - 1; } @Override @@ -77,7 +79,7 @@ public long getNextRecordReference( RECORD record ) public static long longFromIntAndMod( long base, long modifier ) { - return modifier == 0 && base == IdGeneratorImpl.INTEGER_MINUS_ONE ? -1 : base | modifier; + return modifier == 0 && IdValidator.isReservedId( base ) ? -1 : base | modifier; } @Override @@ -98,8 +100,9 @@ public int hashCode() } - protected long getMaxId(int bits) + @Override + public final long getMaxId() { - return (1L << bits) - 1; + return maxId; } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/DynamicRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/DynamicRecordFormat.java index 91e918fce1e1a..2d9bf6475c0c8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/DynamicRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/DynamicRecordFormat.java @@ -28,7 +28,6 @@ import org.neo4j.kernel.impl.store.record.RecordLoad; import static java.lang.String.format; - import static org.neo4j.kernel.impl.store.record.DynamicRecord.NO_DATA; public class DynamicRecordFormat extends BaseOneByteHeaderRecordFormat @@ -38,7 +37,8 @@ public class DynamicRecordFormat extends BaseOneByteHeaderRecordFormat { public LabelTokenRecordFormat() { - super( BASE_RECORD_SIZE ); + super( BASE_RECORD_SIZE, LowLimitFormatSettings.LABEL_TOKEN_MAXIMUM_ID_BITS ); } @Override @@ -33,10 +33,4 @@ public LabelTokenRecord newRecord() { return new LabelTokenRecord( -1 ); } - - @Override - public long getMaxId() - { - return getMaxId( LowLimitFormatSettings.LABEL_TOKEN_MAXIMUM_ID_BITS ); - } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/MetaDataRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/MetaDataRecordFormat.java index 6f97cccf38ac7..8547cd14c269c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/MetaDataRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/MetaDataRecordFormat.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.store.format.lowlimit; import java.io.IOException; + import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; import org.neo4j.kernel.impl.store.MetaDataStore.Position; @@ -32,10 +33,11 @@ public class MetaDataRecordFormat extends BaseOneByteHeaderRecordFormat public NodeRecordFormat() { - super( fixedRecordSize( RECORD_SIZE ), 0, IN_USE_BIT ); + super( fixedRecordSize( RECORD_SIZE ), 0, IN_USE_BIT, LowLimitFormatSettings.NODE_RECORD_MAXIMUM_ID_BITS ); } @Override @@ -45,12 +45,6 @@ public NodeRecord newRecord() return new NodeRecord( -1 ); } - @Override - public long getMaxId() - { - return getMaxId( LowLimitFormatSettings.NODE_RECORD_MAXIMUM_ID_BITS ); - } - public void read( NodeRecord record, PageCursor cursor, RecordLoad mode, int recordSize, PagedFile storeFile ) throws IOException { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/NodeRecordFormatV2_0.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/NodeRecordFormatV2_0.java index 2c61df07b9236..af36ebac519c9 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/NodeRecordFormatV2_0.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/NodeRecordFormatV2_0.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.store.format.lowlimit; import java.io.IOException; + import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; import org.neo4j.kernel.impl.store.format.BaseOneByteHeaderRecordFormat; @@ -30,7 +31,7 @@ class NodeRecordFormatV2_0 extends BaseOneByteHeaderRecordFormat { NodeRecordFormatV2_0() { - super( fixedRecordSize( 14 ), 0, IN_USE_BIT ); + super( fixedRecordSize( 14 ), 0, IN_USE_BIT, LowLimitFormatSettings.NODE_RECORD_MAXIMUM_ID_BITS ); } @Override @@ -39,12 +40,6 @@ public NodeRecord newRecord() return new NodeRecord( -1 ); } - @Override - public long getMaxId() - { - return LowLimitFormatSettings.NODE_RECORD_MAXIMUM_ID_BITS; - } - public void read( NodeRecord record, PageCursor cursor, RecordLoad mode, int recordSize, PagedFile storeFile ) throws IOException { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/PropertyKeyTokenRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/PropertyKeyTokenRecordFormat.java index 6e8779f0910cb..b98741dd77025 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/PropertyKeyTokenRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/PropertyKeyTokenRecordFormat.java @@ -26,7 +26,7 @@ public class PropertyKeyTokenRecordFormat extends TokenRecordFormat public PropertyRecordFormat() { - super( fixedRecordSize( RECORD_SIZE ), 0 ); + super( fixedRecordSize( RECORD_SIZE ), 0, LowLimitFormatSettings.PROPERTY_RECORD_MAXIMUM_ID_BITS ); } @Override @@ -135,12 +135,6 @@ public long getNextRecordReference( PropertyRecord record ) return record.getNextProp(); } - @Override - public long getMaxId() - { - return getMaxId( LowLimitFormatSettings.PROPERTY_RECORD_MAXIMUM_ID_BITS ); - } - /** * For property records there's no "inUse" byte and we need to read the whole record to * see if there are any PropertyBlocks in use in it. diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipGroupRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipGroupRecordFormat.java index c94ddc5a053ce..07323b69edf58 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipGroupRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipGroupRecordFormat.java @@ -42,7 +42,8 @@ public class RelationshipGroupRecordFormat extends BaseOneByteHeaderRecordFormat public RelationshipGroupRecordFormat() { - super( fixedRecordSize( RECORD_SIZE ), 0, IN_USE_BIT ); + super( fixedRecordSize( RECORD_SIZE ), 0, IN_USE_BIT, + LowLimitFormatSettings.RELATIONSHIP_GROUP_MAXIMUM_ID_BITS ); } @Override @@ -126,10 +127,4 @@ public long getNextRecordReference( RelationshipGroupRecord record ) { return record.getNext(); } - - @Override - public long getMaxId() - { - return getMaxId( LowLimitFormatSettings.RELATIONSHIP_GROUP_MAXIMUM_ID_BITS ); - } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipRecordFormat.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipRecordFormat.java index 7ad70c7403bdf..d2ac995ea740c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipRecordFormat.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/lowlimit/RelationshipRecordFormat.java @@ -39,7 +39,7 @@ public class RelationshipRecordFormat extends BaseOneByteHeaderRecordFormat extends Base { protected static final int BASE_RECORD_SIZE = 1/*inUse*/ + 4/*nameId*/; - protected TokenRecordFormat( int recordSize ) + protected TokenRecordFormat( int recordSize, int idBits ) { - super( fixedRecordSize( recordSize ), 0, IN_USE_BIT ); + super( fixedRecordSize( recordSize ), 0, IN_USE_BIT, idBits ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdGeneratorImpl.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdGeneratorImpl.java index 9110d86777d5a..e547754dced96 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdGeneratorImpl.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/IdGeneratorImpl.java @@ -28,6 +28,7 @@ import org.neo4j.io.fs.StoreChannel; import org.neo4j.kernel.impl.store.InvalidIdGeneratorException; import org.neo4j.kernel.impl.store.UnderlyingStorageException; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; import static java.lang.Math.max; @@ -71,6 +72,10 @@ public class IdGeneratorImpl implements IdGenerator private static final byte CLEAN_GENERATOR = (byte) 0; private static final byte STICKY_GENERATOR = (byte) 1; + /** + * Invalid and reserved id value. Represents special values, f.ex. the end of a relationships/property chain. + * Please use {@link IdValidator} to validate generated ids. + */ public static final long INTEGER_MINUS_ONE = 0xFFFFFFFFL; // 4294967295L; // number of defragged ids to grab from file in batch (also used for write) @@ -149,26 +154,15 @@ public synchronized long nextId() } long id = highId.get(); - if ( id == INTEGER_MINUS_ONE ) + if ( IdValidator.isReservedId( id ) ) { - // Skip the integer -1 (0xFFFFFFFF) because it represents - // special values, f.ex. the end of a relationships/property chain. id = highId.incrementAndGet(); } - assertIdWithinCapacity( id ); + IdValidator.assertValidId( id, max ); highId.incrementAndGet(); return id; } - private void assertIdWithinCapacity( long id ) - { - if ( id > max || id < 0 ) - { - throw new UnderlyingStorageException( - "Id capacity exceeded: " + id + " is not within bounds [0; " + max + "] for " + file ); - } - } - private void assertStillOpen() { if ( fileChannel == null ) @@ -215,7 +209,7 @@ public synchronized IdRange nextIdBatch( int size ) @Override public void setHighId( long id ) { - assertIdWithinCapacity( id ); + IdValidator.assertIdWithinCapacity( id, max ); highId.set( id ); } @@ -253,7 +247,7 @@ public long getHighestPossibleIdInUse() @Override public synchronized void freeId( long id ) { - if ( id == INTEGER_MINUS_ONE ) + if ( IdValidator.isReservedId( id ) ) { return; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdCapacityExceededException.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdCapacityExceededException.java new file mode 100644 index 0000000000000..325c0f797167a --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdCapacityExceededException.java @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2002-2016 "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 . + */ +package org.neo4j.kernel.impl.store.id.validation; + +import org.neo4j.kernel.impl.store.UnderlyingStorageException; + +public class IdCapacityExceededException extends UnderlyingStorageException +{ + public IdCapacityExceededException( long id, long maxId ) + { + super( "Record id " + id + " is out of range [0, " + maxId + "]" ); + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdValidator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdValidator.java new file mode 100644 index 0000000000000..21a486f8e4c93 --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/IdValidator.java @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2002-2016 "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 . + */ +package org.neo4j.kernel.impl.store.id.validation; + +import org.neo4j.kernel.impl.store.id.IdGenerator; +import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; +import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; + +/** + * Utility containing methods to validate record id used in {@link AbstractBaseRecord} and possibly generated by + * {@link IdGenerator}. Takes into account special reserved value {@link IdGeneratorImpl#INTEGER_MINUS_ONE}. + */ +public final class IdValidator +{ + private IdValidator() + { + } + + /** + * Checks if the given id is reserved, i.e. {@link IdGeneratorImpl#INTEGER_MINUS_ONE}. + * + * @param id the id to check. + * @return true if the given id is {@link IdGeneratorImpl#INTEGER_MINUS_ONE}, false + * otherwise. + * @see IdGeneratorImpl#INTEGER_MINUS_ONE + */ + public static boolean isReservedId( long id ) + { + return id == IdGeneratorImpl.INTEGER_MINUS_ONE; + } + + /** + * Asserts that the given id is valid: + *
    + *
  • non-negative + *
  • less than the given max id + *
  • not equal to {@link IdGeneratorImpl#INTEGER_MINUS_ONE} + *
+ * + * @param id the id to check. + * @param maxId the max allowed id. + * @see IdGeneratorImpl#INTEGER_MINUS_ONE + */ + public static void assertValidId( long id, long maxId ) + { + if ( isReservedId( id ) ) + { + throw new ReservedIdException( id ); + } + assertIdWithinCapacity( id, maxId ); + } + + /** + * Asserts that the given id is valid with respect to given max id: + *
    + *
  • non-negative + *
  • less than the given max id + *
+ * + * @param id the id to check. + * @param maxId the max allowed id. + */ + public static void assertIdWithinCapacity( long id, long maxId ) + { + if ( id < 0 ) + { + throw new NegativeIdException( id ); + } + if ( id > maxId ) + { + throw new IdCapacityExceededException( id, maxId ); + } + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/NegativeIdException.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/NegativeIdException.java new file mode 100644 index 0000000000000..fa62d243cdb87 --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/NegativeIdException.java @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2002-2016 "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 . + */ +package org.neo4j.kernel.impl.store.id.validation; + +import org.neo4j.kernel.impl.store.UnderlyingStorageException; + +public class NegativeIdException extends UnderlyingStorageException +{ + public NegativeIdException( long id ) + { + super( "Record id can't be negative: " + id ); + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/ReservedIdException.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/ReservedIdException.java new file mode 100644 index 0000000000000..30e339a4cead9 --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/id/validation/ReservedIdException.java @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2002-2016 "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 . + */ +package org.neo4j.kernel.impl.store.id.validation; + +import org.neo4j.kernel.impl.store.UnderlyingStorageException; + +public class ReservedIdException extends UnderlyingStorageException +{ + public ReservedIdException( long id ) + { + super( "Id " + id + " is reserved and can't be used as a regular record id" ); + } +} diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java b/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java index dd34a7a383b35..c40e0ad8b0b0d 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java @@ -116,7 +116,7 @@ import org.neo4j.kernel.impl.store.format.RecordFormats; import org.neo4j.kernel.impl.store.id.DefaultIdGeneratorFactory; import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; -import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; import org.neo4j.kernel.impl.store.record.DynamicRecord; import org.neo4j.kernel.impl.store.record.IndexRule; import org.neo4j.kernel.impl.store.record.LabelTokenRecord; @@ -796,14 +796,7 @@ private boolean arrayContains( long[] ids, int cursor, int labelId ) @Override public void createNode( long id, Map properties, Label... labels ) { - if ( id < 0 || id > maxNodeId ) - { - throw new IllegalArgumentException( "id=" + id ); - } - if ( id == IdGeneratorImpl.INTEGER_MINUS_ONE ) - { - throw new IllegalArgumentException( "id " + id + " is reserved for internal use" ); - } + IdValidator.assertValidId( id, maxNodeId ); if ( nodeStore.isInUse( id ) ) { throw new IllegalArgumentException( "id=" + id + " already in use" ); diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdSequence.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdSequence.java index 96c7bddfadf4d..c6f5219a5722b 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdSequence.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingIdSequence.java @@ -19,8 +19,8 @@ */ package org.neo4j.unsafe.impl.batchimport.store; -import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; import org.neo4j.kernel.impl.store.id.IdSequence; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; /** * {@link IdSequence} w/o any synchronization, purely a long incrementing. @@ -61,7 +61,7 @@ public void set( long nextId ) public long peek() { - if ( nextId == IdGeneratorImpl.INTEGER_MINUS_ONE ) + if ( IdValidator.isReservedId( nextId ) ) { nextId++; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/JumpingIdGeneratorFactory.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/JumpingIdGeneratorFactory.java index da748d547538f..3ae125f15dd40 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/JumpingIdGeneratorFactory.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/JumpingIdGeneratorFactory.java @@ -24,11 +24,11 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicLong; -import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; -import org.neo4j.kernel.impl.store.id.IdType; import org.neo4j.kernel.impl.store.id.IdGenerator; -import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; +import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; import org.neo4j.kernel.impl.store.id.IdRange; +import org.neo4j.kernel.impl.store.id.IdType; +import org.neo4j.kernel.impl.store.id.validation.IdValidator; import org.neo4j.test.impl.EphemeralIdGenerator; public class JumpingIdGeneratorFactory implements IdGeneratorFactory @@ -92,7 +92,7 @@ public long nextId() private long tryNextId() { long result = nextId.getAndIncrement(); - if ( result == IdGeneratorImpl.INTEGER_MINUS_ONE ) + if ( IdValidator.isReservedId( result ) ) { result = nextId.getAndIncrement(); leftToNextJump--; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java index 2f7a2e48154c2..51b538f718148 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/CommonAbstractStoreTest.java @@ -41,7 +41,11 @@ import org.neo4j.kernel.impl.store.id.DefaultIdGeneratorFactory; import org.neo4j.kernel.impl.store.id.IdGenerator; import org.neo4j.kernel.impl.store.id.IdGeneratorFactory; +import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; import org.neo4j.kernel.impl.store.id.IdType; +import org.neo4j.kernel.impl.store.id.validation.IdCapacityExceededException; +import org.neo4j.kernel.impl.store.id.validation.NegativeIdException; +import org.neo4j.kernel.impl.store.id.validation.ReservedIdException; import org.neo4j.kernel.impl.store.record.AbstractBaseRecord; import org.neo4j.kernel.impl.store.record.NodeRecord; import org.neo4j.kernel.impl.store.record.RecordLoad; @@ -50,9 +54,12 @@ import org.neo4j.test.PageCacheRule; import org.neo4j.test.TargetDirectory; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; @@ -79,6 +86,7 @@ public class CommonAbstractStoreTest private final PageCache pageCache = mock( PageCache.class ); private final Config config = Config.empty(); private final File storeFile = new File( "store" ); + private RecordFormat recordFormat = mock( RecordFormat.class ); private final IdType idType = IdType.RELATIONSHIP; // whatever private static final FileSystemAbstraction fs = new DefaultFileSystemAbstraction(); @@ -161,10 +169,62 @@ public void recordCursorPinsEachPageItReads() throws Exception } } - @SuppressWarnings( "unchecked" ) + @Test + public void throwsWhenRecordWithNegativeIdIsUpdated() + { + TheStore store = newStore(); + TheRecord record = new TheRecord( -1 ); + + try + { + store.updateRecord( record ); + fail( "Should have failed" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( NegativeIdException.class ) ); + } + } + + @Test + public void throwsWhenRecordWithTooHighIdIsUpdated() + { + long maxFormatId = 42; + when( recordFormat.getMaxId() ).thenReturn( maxFormatId ); + + TheStore store = newStore(); + TheRecord record = new TheRecord( maxFormatId + 1 ); + + try + { + store.updateRecord( record ); + fail( "Should have failed" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( IdCapacityExceededException.class ) ); + } + } + + @Test + public void throwsWhenRecordWithReservedIdIsUpdated() + { + TheStore store = newStore(); + TheRecord record = new TheRecord( IdGeneratorImpl.INTEGER_MINUS_ONE ); + + try + { + store.updateRecord( record ); + fail( "Should have failed" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( ReservedIdException.class ) ); + } + } + private TheStore newStore() { - RecordFormat recordFormat = mock( RecordFormat.class ); LogProvider log = NullLogProvider.getInstance(); TheStore store = new TheStore( storeFile, config, idType, idGeneratorFactory, pageCache, log, recordFormat ); store.initialise( false ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/LowLimitTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/LowLimitV3_0Test.java similarity index 97% rename from community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/LowLimitTest.java rename to community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/LowLimitV3_0Test.java index 7b5f181ea48a2..e10392cc849ae 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/LowLimitTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/lowlimit/LowLimitV3_0Test.java @@ -27,7 +27,7 @@ import static org.junit.Assert.assertEquals; import static org.neo4j.kernel.impl.store.format.InternalRecordFormatSelector.select; -public class LowLimitTest +public class LowLimitV3_0Test { @Test public void shouldResolveLowLimitsRecordFormat() throws Exception diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdGeneratorImplTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdGeneratorImplTest.java index 08ab2857b64de..71096e4b4f3bb 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdGeneratorImplTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/id/IdGeneratorImplTest.java @@ -27,18 +27,18 @@ import org.neo4j.io.fs.DefaultFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.kernel.impl.store.InvalidIdGeneratorException; -import org.neo4j.kernel.impl.store.UnderlyingStorageException; +import org.neo4j.kernel.impl.store.id.validation.IdCapacityExceededException; +import org.neo4j.kernel.impl.store.id.validation.NegativeIdException; import org.neo4j.test.EphemeralFileSystemRule; +import static java.util.concurrent.TimeUnit.MINUTES; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; - -import static java.util.concurrent.TimeUnit.MINUTES; - import static org.neo4j.test.ProcessTestUtil.executeSubProcess; public class IdGeneratorImplTest @@ -60,11 +60,68 @@ public void shouldNotAcceptMinusOne() throws Exception idGenerator.setHighId( -1 ); fail( "Should have failed" ); } - catch ( UnderlyingStorageException e ) - { // OK + catch ( Exception e ) + { + assertThat( e, instanceOf( NegativeIdException.class ) ); + } + } + + @Test + public void throwsWhenNextIdIsTooHigh() + { + long maxId = 10; + IdGeneratorImpl.createGenerator( fsr.get(), file, 0, false ); + IdGenerator idGenerator = new IdGeneratorImpl( fsr.get(), file, 1, maxId, false, 0 ); + + for ( long i = 0; i <= maxId; i++ ) + { + idGenerator.nextId(); + } + + try + { + idGenerator.nextId(); + fail( "Should have failed" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( IdCapacityExceededException.class ) ); } } + @Test + public void throwsWhenGivenHighIdIsTooHigh() + { + long maxId = 10; + IdGeneratorImpl.createGenerator( fsr.get(), file, 0, false ); + IdGenerator idGenerator = new IdGeneratorImpl( fsr.get(), file, 1, maxId, false, 0 ); + + try + { + idGenerator.setHighId( maxId + 1 ); + fail( "Should have failed" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( IdCapacityExceededException.class ) ); + } + } + + /** + * It should be fine to set high id to {@link IdGeneratorImpl#INTEGER_MINUS_ONE}. + * It will just be never returned from {@link IdGeneratorImpl#nextId()}. + */ + @Test + public void highIdCouldBeSetToReservedId() + { + IdGeneratorImpl.createGenerator( fsr.get(), file, 0, false ); + IdGenerator idGenerator = new IdGeneratorImpl( fsr.get(), file, 1, Long.MAX_VALUE, false, 0 ); + + idGenerator.setHighId( IdGeneratorImpl.INTEGER_MINUS_ONE ); + + assertEquals( IdGeneratorImpl.INTEGER_MINUS_ONE + 1, idGenerator.nextId() ); + } + @Test public void correctDefragCountWhenHaveIdsInFile() { diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/BigBatchStoreIT.java b/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/BigBatchStoreIT.java index 80281a43b1eb8..3cfd046ad61f8 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/BigBatchStoreIT.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/BigBatchStoreIT.java @@ -34,7 +34,9 @@ import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.RelationshipType; import org.neo4j.helpers.collection.Iterables; +import org.neo4j.kernel.impl.store.id.IdGeneratorImpl; import org.neo4j.kernel.impl.store.id.IdType; +import org.neo4j.kernel.impl.store.id.validation.ReservedIdException; import org.neo4j.test.EphemeralFileSystemRule; import org.neo4j.test.TestGraphDatabaseFactory; import org.neo4j.unsafe.batchinsert.internal.BatchInserterImpl; @@ -135,10 +137,10 @@ private void testHighIds( long highMark, int minus, int requiredHeapMb ) throws db = BatchInserters.inserter( PATH.getAbsoluteFile(), fs.get() ); } - @Test( expected=IllegalArgumentException.class ) + @Test( expected = ReservedIdException.class ) public void makeSureCantCreateNodeWithMagicNumber() { - long id = (long) Math.pow( 2, 32 )-1; + long id = IdGeneratorImpl.INTEGER_MINUS_ONE; db.createNode( id, null ); } diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/BaseHighLimitRecordFormat.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/BaseHighLimitRecordFormat.java index bb51c7cc66103..bceda4f9cc315 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/BaseHighLimitRecordFormat.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/BaseHighLimitRecordFormat.java @@ -88,13 +88,7 @@ abstract class BaseHighLimitRecordFormat protected BaseHighLimitRecordFormat( Function recordSize, int recordHeaderSize ) { - super( recordSize, recordHeaderSize, IN_USE_BIT ); - } - - @Override - public long getMaxId() - { - return getMaxId( HighLimit.DEFAULT_MAXIMUM_BITS_PER_ID ); + super( recordSize, recordHeaderSize, IN_USE_BIT, HighLimit.DEFAULT_MAXIMUM_BITS_PER_ID ); } public void read( RECORD record, PageCursor primaryCursor, RecordLoad mode, int recordSize, PagedFile storeFile ) diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/DynamicRecordFormat.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/DynamicRecordFormat.java index 953bd195be15b..5c4157c3c66f6 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/DynamicRecordFormat.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/DynamicRecordFormat.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.store.format.highlimit; import java.io.IOException; + import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; import org.neo4j.kernel.impl.store.format.BaseOneByteHeaderRecordFormat; @@ -49,7 +50,7 @@ class DynamicRecordFormat extends BaseOneByteHeaderRecordFormat protected DynamicRecordFormat() { - super( INT_STORE_HEADER_READER, RECORD_HEADER_SIZE, IN_USE_BIT ); + super( INT_STORE_HEADER_READER, RECORD_HEADER_SIZE, IN_USE_BIT, HighLimit.DEFAULT_MAXIMUM_BITS_PER_ID ); } @Override @@ -99,10 +100,4 @@ public long getNextRecordReference( DynamicRecord record ) { return record.getNextBlock(); } - - @Override - public long getMaxId() - { - return getMaxId( HighLimit.DEFAULT_MAXIMUM_BITS_PER_ID ); - } } diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/PropertyRecordFormat.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/PropertyRecordFormat.java index fc4e490008212..dcd28db91be7c 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/PropertyRecordFormat.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/PropertyRecordFormat.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.store.format.highlimit; import java.io.IOException; + import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; import org.neo4j.kernel.impl.store.format.BaseOneByteHeaderRecordFormat; @@ -46,13 +47,14 @@ * 8B property block * * => 39B-49B - */class PropertyRecordFormat extends BaseOneByteHeaderRecordFormat + */ +class PropertyRecordFormat extends BaseOneByteHeaderRecordFormat { private static final int RECORD_SIZE = 48; protected PropertyRecordFormat() { - super( fixedRecordSize( RECORD_SIZE ), 0, IN_USE_BIT ); + super( fixedRecordSize( RECORD_SIZE ), 0, IN_USE_BIT, HighLimit.DEFAULT_MAXIMUM_BITS_PER_ID ); } @Override @@ -120,10 +122,4 @@ public long getNextRecordReference( PropertyRecord record ) { return record.getNextProp(); } - - @Override - public long getMaxId() - { - return getMaxId( HighLimit.DEFAULT_MAXIMUM_BITS_PER_ID ); - } }