From 102120013bea60005116802176dce369b3bf965b Mon Sep 17 00:00:00 2001 From: Tsvetelin Pantev Date: Tue, 17 Mar 2026 19:26:21 +0100 Subject: [PATCH 1/3] Fix ClassCastException in ConstraintChecker.STORAGE_READER_CONSTRAINT_BUILDER Cast constraintDescriptor.schema() instead of constraintDescriptor itself to LabelSchemaDescriptor/RelationTypeSchemaDescriptor. The previous code threw ClassCastException at transaction commit time for any database with property existence constraints, since ConstraintDescriptorImplementation is not a LabelSchemaDescriptor or RelationTypeSchemaDescriptor. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../impl/constraints/ConstraintChecker.java | 4 +- .../constraints/ConstraintCheckerTest.java | 48 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) 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..73b6972 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,11 @@ 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..9d1d865 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 @@ -15,6 +15,7 @@ import static org.neo4j.kernel.impl.constraints.ConstraintChecker.STORAGE_READER_CONSTRAINT_BUILDER; import java.util.Collections; +import java.util.List; import org.eclipse.collections.api.set.primitive.IntSet; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -50,4 +51,51 @@ public void testCheckNode_NoViolations() throws NodePropertyExistenceException { // Ideally, no exception should be thrown if no constraint is violated assertDoesNotThrow(() -> checker.checkNode(1, mock(TokenSet.class), mock(IntSet.class))); } + + @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. + LabelSchemaDescriptor labelSchema = mock(LabelSchemaDescriptor.class); + when(labelSchema.isSchemaDescriptorType(LabelSchemaDescriptor.class)).thenReturn(true); + when(labelSchema.isSchemaDescriptorType(RelationTypeSchemaDescriptor.class)).thenReturn(false); + when(labelSchema.getLabelId()).thenReturn(1); + when(labelSchema.getPropertyIds()).thenReturn(new int[] {10}); + + ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); + when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(true); + when(constraintDescriptor.schema()).thenReturn(labelSchema); + + when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraintDescriptor).iterator()); + + ConstraintChecker result = assertDoesNotThrow(() -> STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock)); + + assertNotSame(EMPTY_CHECKER, result); + assertEquals(1, result.getNodeLabelSchemaDescriptors().size()); + assertSame(labelSchema, result.getNodeLabelSchemaDescriptors().get(0)); + assertTrue(result.getRelationTypeSchemaDescriptors().isEmpty()); + } + + @Test + public void testStorageReaderConstraintBuilder_RelPropertyExistenceConstraint() { + RelationTypeSchemaDescriptor relSchema = mock(RelationTypeSchemaDescriptor.class); + when(relSchema.isSchemaDescriptorType(LabelSchemaDescriptor.class)).thenReturn(false); + when(relSchema.isSchemaDescriptorType(RelationTypeSchemaDescriptor.class)).thenReturn(true); + when(relSchema.getRelTypeId()).thenReturn(2); + when(relSchema.getPropertyIds()).thenReturn(new int[] {20}); + + ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); + when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(true); + when(constraintDescriptor.schema()).thenReturn(relSchema); + + when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraintDescriptor).iterator()); + + ConstraintChecker result = assertDoesNotThrow(() -> STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock)); + + assertNotSame(EMPTY_CHECKER, result); + assertTrue(result.getNodeLabelSchemaDescriptors().isEmpty()); + assertEquals(1, result.getRelationTypeSchemaDescriptors().size()); + assertSame(relSchema, result.getRelationTypeSchemaDescriptors().get(0)); + } } From cc4d063bec2337de10ea1e4f7794b6c51e73bcdb Mon Sep 17 00:00:00 2001 From: Tsvetelin Pantev Date: Wed, 18 Mar 2026 10:56:55 +0100 Subject: [PATCH 2/3] Improve ConstraintChecker tests: use real objects and test constraint enforcement Use SchemaDescriptors/ConstraintDescriptorFactory instead of mocks for schema and constraint descriptors. Add tests for checkNode violation detection, positive pass-through, label mismatch, and non-existence constraint filtering. Switch assertions to AssertJ for codebase consistency. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../constraints/ConstraintCheckerTest.java | 135 +++++++++++++----- 1 file changed, 97 insertions(+), 38 deletions(-) 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 9d1d865..cd949dc 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,18 +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 java.util.List; -import org.eclipse.collections.api.set.primitive.IntSet; +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; @@ -39,17 +42,19 @@ 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 { - ConstraintChecker checker = - new ConstraintChecker(storageReaderMock, Collections.emptyList(), Collections.emptyList()); + public void testStorageReaderConstraintBuilder_SkipsNonExistenceConstraints() { + ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); + when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(false); - // Ideally, no exception should be thrown if no constraint is violated - assertDoesNotThrow(() -> checker.checkNode(1, mock(TokenSet.class), mock(IntSet.class))); + when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraintDescriptor).iterator()); + + ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); + + assertThat(result).isSameAs(EMPTY_CHECKER); } @Test @@ -57,45 +62,99 @@ 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. - LabelSchemaDescriptor labelSchema = mock(LabelSchemaDescriptor.class); - when(labelSchema.isSchemaDescriptorType(LabelSchemaDescriptor.class)).thenReturn(true); - when(labelSchema.isSchemaDescriptorType(RelationTypeSchemaDescriptor.class)).thenReturn(false); - when(labelSchema.getLabelId()).thenReturn(1); - when(labelSchema.getPropertyIds()).thenReturn(new int[] {10}); - - ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); - when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(true); - when(constraintDescriptor.schema()).thenReturn(labelSchema); + int labelId = 1; + int propertyId = 10; + LabelSchemaDescriptor labelSchema = SchemaDescriptors.forLabel(labelId, propertyId); + ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(labelSchema, false); - when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraintDescriptor).iterator()); + when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraint).iterator()); - ConstraintChecker result = assertDoesNotThrow(() -> STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock)); + ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); - assertNotSame(EMPTY_CHECKER, result); - assertEquals(1, result.getNodeLabelSchemaDescriptors().size()); - assertSame(labelSchema, result.getNodeLabelSchemaDescriptors().get(0)); - assertTrue(result.getRelationTypeSchemaDescriptors().isEmpty()); + 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() { - RelationTypeSchemaDescriptor relSchema = mock(RelationTypeSchemaDescriptor.class); - when(relSchema.isSchemaDescriptorType(LabelSchemaDescriptor.class)).thenReturn(false); - when(relSchema.isSchemaDescriptorType(RelationTypeSchemaDescriptor.class)).thenReturn(true); - when(relSchema.getRelTypeId()).thenReturn(2); - when(relSchema.getPropertyIds()).thenReturn(new int[] {20}); + int relTypeId = 2; + int propertyId = 20; + RelationTypeSchemaDescriptor relSchema = SchemaDescriptors.forRelType(relTypeId, propertyId); + ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(relSchema, false); - ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); - when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(true); - when(constraintDescriptor.schema()).thenReturn(relSchema); + when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraint).iterator()); - when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraintDescriptor).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, 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()); - ConstraintChecker result = assertDoesNotThrow(() -> STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock)); + // 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); - assertNotSame(EMPTY_CHECKER, result); - assertTrue(result.getNodeLabelSchemaDescriptors().isEmpty()); - assertEquals(1, result.getRelationTypeSchemaDescriptors().size()); - assertSame(relSchema, result.getRelationTypeSchemaDescriptors().get(0)); + checker.checkNode(1L, tokenSet, IntSets.immutable.empty()); } } From 22f99f9b18dcb1f8199c4d59f372400fa98c7c61 Mon Sep 17 00:00:00 2001 From: Tsvetelin Pantev Date: Wed, 18 Mar 2026 11:13:55 +0100 Subject: [PATCH 3/3] Apply spotless formatting Co-Authored-By: Claude Opus 4.6 (1M context) --- .../impl/constraints/ConstraintChecker.java | 3 ++- .../constraints/ConstraintCheckerTest.java | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) 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 73b6972..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 @@ -83,7 +83,8 @@ TxStateVisitor visit( } if (constraintDescriptor.schema().isSchemaDescriptorType(RelationTypeSchemaDescriptor.class)) { - relsLabelSchemaDescriptors.add((RelationTypeSchemaDescriptor) constraintDescriptor.schema()); + 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 cd949dc..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 @@ -50,7 +50,8 @@ public void testStorageReaderConstraintBuilder_SkipsNonExistenceConstraints() { ConstraintDescriptor constraintDescriptor = mock(ConstraintDescriptor.class); when(constraintDescriptor.enforcesPropertyExistence()).thenReturn(false); - when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraintDescriptor).iterator()); + when(storageReaderMock.constraintsGetAll()) + .thenReturn(List.of(constraintDescriptor).iterator()); ConstraintChecker result = STORAGE_READER_CONSTRAINT_BUILDER.apply(storageReaderMock); @@ -67,14 +68,16 @@ public void testStorageReaderConstraintBuilder_NodePropertyExistenceConstraint() LabelSchemaDescriptor labelSchema = SchemaDescriptors.forLabel(labelId, propertyId); ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(labelSchema, false); - when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraint).iterator()); + 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.getNodeLabelSchemaDescriptors().get(0).getPropertyIds()) + .containsExactly(propertyId); assertThat(result.getRelationTypeSchemaDescriptors()).isEmpty(); assertThat(result.getNodePropertyMap().containsKey(labelId)).isTrue(); assertThat(result.getNodePropertyMap().get(labelId)).containsExactly(propertyId); @@ -87,15 +90,18 @@ public void testStorageReaderConstraintBuilder_RelPropertyExistenceConstraint() RelationTypeSchemaDescriptor relSchema = SchemaDescriptors.forRelType(relTypeId, propertyId); ConstraintDescriptor constraint = ConstraintDescriptorFactory.existsForSchema(relSchema, false); - when(storageReaderMock.constraintsGetAll()).thenReturn(List.of(constraint).iterator()); + 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.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); }