Skip to content

Commit

Permalink
Cleaned up SchemaCache, and reduced lambda function usage
Browse files Browse the repository at this point in the history
  • Loading branch information
@fickludd
fickludd committed Jan 30, 2017
1 parent fa25098 commit feb6d37
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 94 deletions.
87 changes: 42 additions & 45 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SchemaCache.java
View file
Expand Up @@ -25,8 +25,8 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.neo4j.helpers.collection.Iterables;
import org.neo4j.helpers.collection.Iterators;
import org.neo4j.kernel.api.constraints.NodePropertyConstraint;
import org.neo4j.kernel.api.constraints.PropertyConstraint;
Expand All @@ -36,15 +36,13 @@
import org.neo4j.kernel.api.schema.NodePropertyDescriptor;
import org.neo4j.kernel.api.schema.RelationshipPropertyDescriptor;
import org.neo4j.kernel.api.schema_new.SchemaDescriptor;
import org.neo4j.kernel.api.schema_new.SchemaDescriptorPredicates;
import org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptor;
import org.neo4j.kernel.api.schema_new.index.IndexBoundary;
import org.neo4j.kernel.impl.constraints.ConstraintSemantics;
import org.neo4j.kernel.impl.store.record.ConstraintRule;
import org.neo4j.kernel.impl.store.record.IndexRule;
import org.neo4j.storageengine.api.schema.SchemaRule;

import static org.neo4j.helpers.collection.Iterables.filter;

/**
* A cache of {@link SchemaRule schema rules} as well as enforcement of schema consistency.
* Will always reflect the committed state of the schema store.
Expand All @@ -56,11 +54,12 @@
*/
public class SchemaCache
{
private final Map<Long, SchemaRule> rulesByIdMap = new HashMap<>();
private final Map<Long, IndexRule> indexRuleById = new HashMap<>();
private final Map<Long, ConstraintRule> constraintRuleById = new HashMap<>();

private final Collection<NodePropertyConstraint> nodeConstraints = new HashSet<>();
private final Collection<RelationshipPropertyConstraint> relationshipConstraints = new HashSet<>();
private final Map<IndexDescriptor, Long> indexDescriptorCommitIds = new HashMap<>();
private final Set<IndexDescriptor> indexDescriptors = new HashSet<>();
private final ConstraintSemantics constraintSemantics;

public SchemaCache( ConstraintSemantics constraintSemantics, Iterable<SchemaRule> initialRules )
Expand All @@ -77,38 +76,43 @@ private void splitUpInitialRules( Iterable<SchemaRule> initialRules )
}
}

public Iterable<SchemaRule> schemaRules()
{
return rulesByIdMap.values();
}
// SCHEMA RULES

public Iterable<IndexRule> indexRules()
{
return Iterables.map( IndexRule.class::cast,
Iterables.filter( IndexRule.class::isInstance, schemaRules() ) );
return indexRuleById.values();
}

public Iterable<ConstraintRule> constraintRules()
{
return Iterables.map( ConstraintRule.class::cast,
Iterables.filter( ConstraintRule.class::isInstance, schemaRules() ) );
return constraintRuleById.values();
}

public Iterable<SchemaRule> schemaRulesBySchema( SchemaDescriptor descriptor )
public boolean hasConstraintRule( ConstraintDescriptor descriptor )
{
return Iterables.filter( rule -> rule.getSchemaDescriptor().equals( descriptor ), schemaRules() );
for ( ConstraintRule rule : constraintRuleById.values() )
{
if ( rule.getConstraintDescriptor().equals( descriptor ) )
{
return true;
}
}
return false;
}

public Iterable<SchemaRule> schemaRulesForLabel( final int label )
public boolean hasIndexRule( SchemaDescriptor descriptor )
{
return filter( schemaRule -> SchemaDescriptorPredicates.hasLabel( schemaRule, label ), schemaRules() );
for ( IndexRule rule : indexRuleById.values() )
{
if ( rule.getSchemaDescriptor().equals( descriptor ) )
{
return true;
}
}
return false;
}

public Iterable<SchemaRule> schemaRulesForRelationshipType( final int relTypeId )
{
// warning: this previously only worked for rel type existence constraints
return filter( schemaRule -> SchemaDescriptorPredicates.hasRelType( schemaRule, relTypeId ), schemaRules() );
}
// CONSTRAINTS

public Iterator<PropertyConstraint> constraints()
{
Expand Down Expand Up @@ -146,11 +150,11 @@ public Iterator<RelationshipPropertyConstraint> constraintsForRelationshipTypeAn

public void addSchemaRule( SchemaRule rule )
{
rulesByIdMap.put( rule.getId(), rule );

if ( rule instanceof ConstraintRule )
{
PropertyConstraint constraint = constraintSemantics.readConstraint( (ConstraintRule) rule );
ConstraintRule constraintRule = (ConstraintRule) rule;
constraintRuleById.put( constraintRule.getId(), constraintRule );
PropertyConstraint constraint = constraintSemantics.readConstraint( constraintRule );
if ( constraint instanceof NodePropertyConstraint )
{
nodeConstraints.add( (NodePropertyConstraint) constraint );
Expand All @@ -163,16 +167,18 @@ else if ( constraint instanceof RelationshipPropertyConstraint )
else if ( rule instanceof IndexRule )
{
IndexRule indexRule = (IndexRule) rule;
indexDescriptorCommitIds.put( IndexBoundary.map( indexRule.getIndexDescriptor() ), indexRule.getId() );
indexRuleById.put( indexRule.getId(), indexRule );
indexDescriptors.add( IndexBoundary.map( indexRule.getIndexDescriptor() ) );
}
}

public void clear()
{
rulesByIdMap.clear();
indexRuleById.clear();
constraintRuleById.clear();
nodeConstraints.clear();
relationshipConstraints.clear();
indexDescriptorCommitIds.clear();
indexDescriptors.clear();
}

public void load( List<SchemaRule> schemaRuleIterator )
Expand All @@ -186,15 +192,10 @@ public void load( List<SchemaRule> schemaRuleIterator )

public void removeSchemaRule( long id )
{
SchemaRule rule = rulesByIdMap.remove( id );
if ( rule == null )
if ( constraintRuleById.containsKey( id ) )
{
return;
}

if ( rule instanceof ConstraintRule )
{
PropertyConstraint constraint = constraintSemantics.readConstraint( (ConstraintRule) rule );
ConstraintRule rule = constraintRuleById.remove( id );
PropertyConstraint constraint = constraintSemantics.readConstraint( rule );
if ( constraint instanceof NodePropertyConstraint )
{
nodeConstraints.remove( constraint );
Expand All @@ -204,20 +205,16 @@ else if ( constraint instanceof RelationshipPropertyConstraint )
relationshipConstraints.remove( constraint );
}
}
else if ( rule instanceof IndexRule )
else if ( indexRuleById.containsKey( id ) )
{
IndexRule indexRule = (IndexRule) rule;
indexDescriptorCommitIds.remove( IndexBoundary.map( indexRule.getIndexDescriptor() ) );
IndexRule rule = indexRuleById.remove( id );
indexDescriptors.remove( IndexBoundary.map( rule.getIndexDescriptor() ) );
}
}

public IndexDescriptor indexDescriptor( NodePropertyDescriptor descriptor )
{
IndexDescriptor key = IndexDescriptorFactory.of( descriptor );
if ( indexDescriptorCommitIds.containsKey( key ) )
{
return key;
}
return null;
return indexDescriptors.contains( key ) ? key : null;
}
}
40 changes: 13 additions & 27 deletions community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java
View file
Expand Up @@ -69,8 +69,8 @@
import org.neo4j.kernel.api.schema.IndexDescriptorFactory;
import org.neo4j.kernel.api.schema.NodePropertyDescriptor;
import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor;
import org.neo4j.kernel.api.schema_new.RelationTypeSchemaDescriptor;
import org.neo4j.kernel.api.schema_new.SchemaDescriptorFactory;
import org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptor;
import org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptorFactory;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptorFactory;
import org.neo4j.kernel.configuration.Config;
Expand Down Expand Up @@ -162,7 +162,6 @@
import static org.neo4j.collection.primitive.PrimitiveLongCollections.map;
import static org.neo4j.graphdb.Label.label;
import static org.neo4j.kernel.api.schema.IndexDescriptorFactory.getNodePropertyDescriptor;
import static org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptor.Type.EXISTS;
import static org.neo4j.kernel.impl.store.NodeLabelsField.parseLabelsField;
import static org.neo4j.kernel.impl.store.PropertyStore.encodeString;
import static org.neo4j.kernel.impl.util.IoPrimitiveUtils.safeCastLongToInt;
Expand Down Expand Up @@ -389,45 +388,32 @@ private void validateUniquenessConstraintCanBeCreated( int labelId, int[] proper

private void verifyIndexOrUniquenessConstraintCanBeCreated( int labelId, int[] propertyKeyIds, String errorMessage )
{
LabelSchemaDescriptor descriptor = SchemaDescriptorFactory.forLabel( labelId, propertyKeyIds );
for ( SchemaRule rule : schemaCache.schemaRulesBySchema( descriptor ) )
{
if ( !isExistenceConstraint( rule ) )
{
throw new ConstraintViolationException( errorMessage );
}
LabelSchemaDescriptor schemaDescriptor = SchemaDescriptorFactory.forLabel( labelId, propertyKeyIds );
ConstraintDescriptor constraintDescriptor = ConstraintDescriptorFactory.uniqueForLabel( labelId, propertyKeyIds );
if ( schemaCache.hasIndexRule( schemaDescriptor ) || schemaCache.hasConstraintRule( constraintDescriptor ) ) {
throw new ConstraintViolationException( errorMessage );
}
}

private boolean isExistenceConstraint( SchemaRule rule )
{
return rule instanceof ConstraintRule &&
((ConstraintRule)rule).getConstraintDescriptor().type() == EXISTS;
}

private void validateNodePropertyExistenceConstraintCanBeCreated( int labelId, int[] propertyKeyIds )
{
LabelSchemaDescriptor descriptor = SchemaDescriptorFactory.forLabel( labelId, propertyKeyIds );
for ( SchemaRule rule : schemaCache.schemaRulesBySchema( descriptor ) )
ConstraintDescriptor constraintDescriptor = ConstraintDescriptorFactory.existsForLabel( labelId, propertyKeyIds );

if ( schemaCache.hasConstraintRule( constraintDescriptor ) )
{
if ( isExistenceConstraint( rule ) )
{
throw new ConstraintViolationException(
throw new ConstraintViolationException(
"Node property existence constraint for given {label;property} already exists" );
}
}
}

private void validateRelationshipConstraintCanBeCreated( int relTypeId, int propertyKeyId )
{
RelationTypeSchemaDescriptor descriptor = SchemaDescriptorFactory.forRelType( relTypeId, propertyKeyId );
for ( SchemaRule rule : schemaCache.schemaRulesBySchema( descriptor ) )
ConstraintDescriptor constraintDescriptor = ConstraintDescriptorFactory.existsForLabel( relTypeId, propertyKeyId );

if ( schemaCache.hasConstraintRule( constraintDescriptor ) )
{
if ( isExistenceConstraint( rule ) )
{
throw new ConstraintViolationException(
throw new ConstraintViolationException(
"Relationship property existence constraint for given {type;property} already exists" );
}
}
}

Expand Down
30 changes: 8 additions & 22 deletions community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/SchemaCacheTest.java
View file
Expand Up @@ -56,49 +56,35 @@
public class SchemaCacheTest
{
final SchemaRule hans = newIndexRule( 1, 0, 5 );
final SchemaRule witch = newIndexRule( 2, 3, 6 );
final SchemaRule witch = nodePropertyExistenceConstraintRule( 2, 3, 6 );
final SchemaRule gretel = newIndexRule( 3, 0, 7 );

@Test
public void should_construct_schema_cache()
{
// GIVEN
Collection<SchemaRule> rules = asList( hans, witch, gretel );
SchemaCache cache = new SchemaCache( new StandardConstraintSemantics(), rules );
SchemaCache cache = new SchemaCache( new ConstraintSemantics(), rules );

// THEN
assertEquals( asSet( hans, gretel ), Iterables.asSet( cache.schemaRulesForLabel( 0 ) ) );
assertEquals( asSet( witch ), Iterables.asSet( cache.schemaRulesForLabel( 3 ) ) );
assertEquals( Iterables.asSet( rules ), Iterables.asSet( cache.schemaRules() ) );
assertEquals( asSet( hans, gretel ), Iterables.asSet( cache.indexRules() ) );
assertEquals( asSet( witch ), Iterables.asSet( cache.constraintRules() ) );
}

@Test
public void should_add_schema_rules_to_a_label()
{
// GIVEN
Collection<SchemaRule> rules = Collections.emptyList();
SchemaCache cache = new SchemaCache( new StandardConstraintSemantics(), rules );

// WHEN
cache.addSchemaRule( hans );
cache.addSchemaRule( gretel );

// THEN
assertEquals( asSet( hans, gretel ), Iterables.asSet( cache.schemaRulesForLabel( 0 ) ) );
}

@Test
public void should_to_retrieve_all_schema_rules()
public void should_add_schema_rules()
{
// GIVEN
SchemaCache cache = newSchemaCache();

// WHEN
cache.addSchemaRule( hans );
cache.addSchemaRule( gretel );
cache.addSchemaRule( witch );

// THEN
assertEquals( asSet( hans, gretel ), Iterables.asSet( cache.schemaRules() ) );
assertEquals( asSet( hans, gretel ), Iterables.asSet( cache.indexRules() ) );
assertEquals( asSet( witch ), Iterables.asSet( cache.constraintRules() ) );
}

@Test
Expand Down

0 comments on commit feb6d37

Please sign in to comment.