diff --git a/core/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintChecker.java b/core/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintChecker.java index f842c20..89083b8 100644 --- a/core/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintChecker.java +++ b/core/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintChecker.java @@ -79,11 +79,12 @@ TxStateVisitor visit( ConstraintDescriptor constraintDescriptor = it.next(); if (constraintDescriptor.enforcesPropertyExistence()) { if (constraintDescriptor.schema().isSchemaDescriptorType(LabelSchemaDescriptor.class)) { - nodeLabelSchemaDescriptors.add((LabelSchemaDescriptor) constraintDescriptor); + nodeLabelSchemaDescriptors.add((LabelSchemaDescriptor) constraintDescriptor.schema()); } if (constraintDescriptor.schema().isSchemaDescriptorType(RelationTypeSchemaDescriptor.class)) { - relsLabelSchemaDescriptors.add((RelationTypeSchemaDescriptor) constraintDescriptor); + relsLabelSchemaDescriptors.add( + (RelationTypeSchemaDescriptor) constraintDescriptor.schema()); } } } diff --git a/core/src/test/java/org/neo4j/kernel/impl/constraints/ConstraintCheckerTest.java b/core/src/test/java/org/neo4j/kernel/impl/constraints/ConstraintCheckerTest.java index da4f3a2..10992f8 100644 --- a/core/src/test/java/org/neo4j/kernel/impl/constraints/ConstraintCheckerTest.java +++ b/core/src/test/java/org/neo4j/kernel/impl/constraints/ConstraintCheckerTest.java @@ -9,17 +9,21 @@ */ package org.neo4j.kernel.impl.constraints; -import static org.junit.jupiter.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.*; import static org.neo4j.kernel.impl.constraints.ConstraintChecker.EMPTY_CHECKER; import static org.neo4j.kernel.impl.constraints.ConstraintChecker.STORAGE_READER_CONSTRAINT_BUILDER; import java.util.Collections; -import org.eclipse.collections.api.set.primitive.IntSet; +import java.util.List; +import org.eclipse.collections.impl.factory.primitive.IntSets; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.neo4j.common.TokenNameLookup; import org.neo4j.internal.kernel.api.TokenSet; import org.neo4j.internal.schema.*; +import org.neo4j.internal.schema.constraints.ConstraintDescriptorFactory; import org.neo4j.kernel.api.exceptions.schema.NodePropertyExistenceException; import org.neo4j.storageengine.api.StorageReader; @@ -38,16 +42,125 @@ public void testStorageReaderConstraintBuilder_EmptyConstraints() { ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); - // Check if it returns the empty checker when there are no constraints - assertSame(EMPTY_CHECKER, result); + assertThat(result).isSameAs(EMPTY_CHECKER); } @Test - public void testCheckNode_NoViolations() throws NodePropertyExistenceException { + public void testStorageReaderConstraintBuilder_SkipsNonExistenceConstraints() { + ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); + when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(false); + + when(storageReaderMock.constraintsGetAll()) + .thenReturn(List.of(constraintDescriptor).iterator()); + + ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); + + assertThat(result).isSameAs(EMPTY_CHECKER); + } + + @Test + public void testStorageReaderConstraintBuilder_NodePropertyExistenceConstraint() { + // Regression test: STORAGE_READER_CONSTRAINT_BUILDER previously cast constraintDescriptor + // directly to LabelSchemaDescriptor instead of constraintDescriptor.schema(), causing + // a ClassCastException at commit time for databases with property existence constraints. + int labelId = 1; + int propertyId = 10; + LabelSchemaDescriptor labelSchema = SchemaDescriptors.forLabel(labelId, propertyId); + ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(labelSchema, false); + + when(storageReaderMock.constraintsGetAll()) + .thenReturn(List.of(constraint).iterator()); + + ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); + + assertThat(result).isNotSameAs(EMPTY_CHECKER); + assertThat(result.getNodeLabelSchemaDescriptors()).hasSize(1); + assertThat(result.getNodeLabelSchemaDescriptors().get(0).getLabelId()).isEqualTo(labelId); + assertThat(result.getNodeLabelSchemaDescriptors().get(0).getPropertyIds()) + .containsExactly(propertyId); + assertThat(result.getRelationTypeSchemaDescriptors()).isEmpty(); + assertThat(result.getNodePropertyMap().containsKey(labelId)).isTrue(); + assertThat(result.getNodePropertyMap().get(labelId)).containsExactly(propertyId); + } + + @Test + public void testStorageReaderConstraintBuilder_RelPropertyExistenceConstraint() { + int relTypeId = 2; + int propertyId = 20; + RelationTypeSchemaDescriptor relSchema = SchemaDescriptors.forRelType(relTypeId, propertyId); + ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(relSchema, false); + + when(storageReaderMock.constraintsGetAll()) + .thenReturn(List.of(constraint).iterator()); + + ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); + + assertThat(result).isNotSameAs(EMPTY_CHECKER); + assertThat(result.getNodeLabelSchemaDescriptors()).isEmpty(); + assertThat(result.getRelationTypeSchemaDescriptors()).hasSize(1); + assertThat(result.getRelationTypeSchemaDescriptors().get(0).getRelTypeId()) + .isEqualTo(relTypeId); + assertThat(result.getRelationTypeSchemaDescriptors().get(0).getPropertyIds()) + .containsExactly(propertyId); + assertThat(result.getRelPropertyMap().containsKey(relTypeId)).isTrue(); + assertThat(result.getRelPropertyMap().get(relTypeId)).containsExactly(propertyId); + } + + @Test + public void testCheckNode_PassesWhenRequiredPropertyPresent() throws NodePropertyExistenceException { + int labelId = 5; + int requiredPropertyId = 42; + LabelSchemaDescriptor labelSchema = SchemaDescriptors.forLabel(labelId, requiredPropertyId); + ConstraintChecker checker = - new ConstraintChecker(storageReaderMock, Collections.emptyList(), Collections.emptyList()); + new ConstraintChecker(storageReaderMock, List.of(labelSchema), Collections.emptyList()); + + TokenSet tokenSet = mock(TokenSet.class); + when(tokenSet.numberOfTokens()).thenReturn(1); + when(tokenSet.token(0)).thenReturn(labelId); + + checker.checkNode(1L, tokenSet, IntSets.immutable.of(requiredPropertyId)); + } + + @Test + public void testCheckNode_DetectsViolationForMissingRequiredProperty() { + int labelId = 5; + int requiredPropertyId = 42; + LabelSchemaDescriptor labelSchema = SchemaDescriptors.forLabel(labelId, requiredPropertyId); + ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(labelSchema, false); + + TokenNameLookup tokenNameLookup = mock(TokenNameLookup.class); + when(tokenNameLookup.labelGetName(labelId)).thenReturn("TestLabel"); + when(tokenNameLookup.propertyKeyGetName(requiredPropertyId)).thenReturn("requiredProp"); + when(storageReaderMock.constraintsGetForSchema(labelSchema)) + .thenReturn(List.of(constraint).iterator()); + when(storageReaderMock.tokenNameLookup()).thenReturn(tokenNameLookup); + + ConstraintChecker checker = + new ConstraintChecker(storageReaderMock, List.of(labelSchema), Collections.emptyList()); + + TokenSet tokenSet = mock(TokenSet.class); + when(tokenSet.numberOfTokens()).thenReturn(1); + when(tokenSet.token(0)).thenReturn(labelId); + + assertThatThrownBy(() -> checker.checkNode(1L, tokenSet, IntSets.immutable.empty())) + .isInstanceOf(NodePropertyExistenceException.class); + } + + @Test + public void testCheckNode_NoViolationsWhenLabelDoesNotMatch() throws NodePropertyExistenceException { + int constrainedLabelId = 5; + int requiredPropertyId = 42; + LabelSchemaDescriptor labelSchema = SchemaDescriptors.forLabel(constrainedLabelId, requiredPropertyId); + + ConstraintChecker checker = + new ConstraintChecker(storageReaderMock, List.of(labelSchema), Collections.emptyList()); + + // Node has a different label (99), so the constraint on label 5 should not apply + TokenSet tokenSet = mock(TokenSet.class); + when(tokenSet.numberOfTokens()).thenReturn(1); + when(tokenSet.token(0)).thenReturn(99); - // Ideally, no exception should be thrown if no constraint is violated - assertDoesNotThrow(() -> checker.checkNode(1, mock(TokenSet.class), mock(IntSet.class))); + checker.checkNode(1L, tokenSet, IntSets.immutable.empty()); } }