-
-
Notifications
You must be signed in to change notification settings - Fork 91
#2560 fix: ensure consistent exception propagation in ImmutableDocument duing lazy loading #2562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
robfrank
merged 2 commits into
main
from
fix/2560-immutable-document-lazy-loading-exception
Sep 28, 2025
Merged
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
261 changes: 261 additions & 0 deletions
261
...ne/src/test/java/com/arcadedb/database/ImmutableDocumentLazyLoadingInconsistencyTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,261 @@ | ||
| /* | ||
| * Copyright © 2021-present Arcade Data Ltd (info@arcadedata.com) | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| * SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com) | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| package com.arcadedb.database; | ||
|
|
||
| import com.arcadedb.TestHelper; | ||
| import com.arcadedb.event.AfterRecordReadListener; | ||
| import com.arcadedb.schema.DocumentType; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| /** | ||
| * Test to verify the fix for issue #2560 - ImmutableDocument.checkForLazyLoading() consistency. | ||
| * | ||
| * This test verifies that after the fix, the get() method now behaves consistently with other methods | ||
| * (has(), modify(), toJSON(), toMap(), getPropertyNames()) by properly propagating exceptions thrown | ||
| * during lazy loading instead of catching them and returning null. | ||
| * | ||
| * The fix ensures that permission checking workflows that depend on exceptions being thrown during | ||
| * lazy loading now work correctly with the get() method. | ||
| */ | ||
| public class ImmutableDocumentLazyLoadingInconsistencyTest extends TestHelper { | ||
|
|
||
| /** | ||
| * Custom RuntimeException to simulate permission denied scenarios. | ||
| */ | ||
| public static class PermissionDeniedException extends RuntimeException { | ||
| public PermissionDeniedException(String message) { | ||
| super(message); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Test listener that simulates a security check that denies access. | ||
| */ | ||
| public static class SecurityCheckListener implements AfterRecordReadListener { | ||
| private final boolean shouldDenyAccess; | ||
| private final String propertyName; | ||
|
|
||
| public SecurityCheckListener(boolean shouldDenyAccess, String propertyName) { | ||
| this.shouldDenyAccess = shouldDenyAccess; | ||
| this.propertyName = propertyName; | ||
| } | ||
|
|
||
| @Override | ||
| public Record onAfterRead(Record record) { | ||
| if (shouldDenyAccess) { | ||
| throw new PermissionDeniedException("Access denied to property: " + propertyName); | ||
| } | ||
| return record; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void beginTest() { | ||
| database.transaction(() -> { | ||
| final DocumentType type = database.getSchema().createDocumentType("SecurityTest"); | ||
| type.createProperty("publicProperty", com.arcadedb.schema.Type.STRING); | ||
| type.createProperty("secretProperty", com.arcadedb.schema.Type.STRING); | ||
| }); | ||
| } | ||
|
|
||
| @Test | ||
| public void testLazyLoadingConsistencyWithSecurityException() { | ||
| database.transaction(() -> { | ||
| // Create a document with both public and secret properties | ||
| final MutableDocument doc = database.newDocument("SecurityTest"); | ||
| doc.set("publicProperty", "public_value"); | ||
| doc.set("secretProperty", "secret_value"); | ||
| doc.save(); | ||
|
|
||
| final RID documentRid = doc.getIdentity(); | ||
|
|
||
| // Register a security listener that denies access when reading records | ||
| final SecurityCheckListener securityListener = new SecurityCheckListener(true, "secretProperty"); | ||
| database.getEvents().registerListener(securityListener); | ||
|
|
||
| try { | ||
| // Test with an ImmutableDocument created by reading from DB (lazy loading will occur on first access) | ||
| database.commit(); | ||
| database.begin(); | ||
|
|
||
| // Get the document as ImmutableDocument (it will have buffer=null initially) | ||
| final ImmutableDocument immutableDoc = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
|
|
||
| // Test 1: has() method should throw exception (current behavior - correct) | ||
| assertThatThrownBy(() -> immutableDoc.has("secretProperty")) | ||
| .isInstanceOf(PermissionDeniedException.class) | ||
| .hasMessage("Access denied to property: secretProperty"); | ||
|
|
||
| // Test 2: Get another instance for modify() test | ||
| final ImmutableDocument immutableDoc2 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc2.modify()) | ||
| .isInstanceOf(PermissionDeniedException.class) | ||
| .hasMessage("Access denied to property: secretProperty"); | ||
|
|
||
| // Test 3: Get another instance for toJSON() test | ||
| final ImmutableDocument immutableDoc3 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc3.toJSON()) | ||
| .isInstanceOf(PermissionDeniedException.class) | ||
| .hasMessage("Access denied to property: secretProperty"); | ||
|
|
||
| // Test 4: Get another instance for toMap() test | ||
| final ImmutableDocument immutableDoc4 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc4.toMap()) | ||
| .isInstanceOf(PermissionDeniedException.class) | ||
| .hasMessage("Access denied to property: secretProperty"); | ||
|
|
||
| // Test 5: Get another instance for getPropertyNames() test | ||
| final ImmutableDocument immutableDoc5 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc5.getPropertyNames()) | ||
| .isInstanceOf(PermissionDeniedException.class) | ||
| .hasMessage("Access denied to property: secretProperty"); | ||
|
|
||
| // Test 6: get() method now correctly throws exception (FIXED - consistent behavior) | ||
| // After the fix, get() now properly propagates exceptions like other methods | ||
| final ImmutableDocument immutableDoc6 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc6.get("secretProperty")) | ||
| .isInstanceOf(PermissionDeniedException.class) | ||
| .hasMessage("Access denied to property: secretProperty"); | ||
|
|
||
| } finally { | ||
| // Clean up the security listener | ||
| database.getEvents().unregisterListener(securityListener); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @Test | ||
| public void testLazyLoadingConsistencyWithoutSecurityException() { | ||
| database.transaction(() -> { | ||
| // Create a document | ||
| final MutableDocument doc = database.newDocument("SecurityTest"); | ||
| doc.set("publicProperty", "public_value"); | ||
| doc.set("secretProperty", "secret_value"); | ||
| doc.save(); | ||
|
|
||
| final RID documentRid = doc.getIdentity(); | ||
|
|
||
| // Register a security listener that allows access | ||
| final SecurityCheckListener securityListener = new SecurityCheckListener(false, "secretProperty"); | ||
| database.getEvents().registerListener(securityListener); | ||
|
|
||
| try { | ||
| database.commit(); | ||
| database.begin(); | ||
|
|
||
| // All methods should work normally when security check passes | ||
| final ImmutableDocument immutableDoc1 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThat(immutableDoc1.has("secretProperty")).isTrue(); | ||
|
|
||
| final ImmutableDocument immutableDoc2 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThat(immutableDoc2.get("secretProperty")).isEqualTo("secret_value"); | ||
|
|
||
| final ImmutableDocument immutableDoc3 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThat(immutableDoc3.modify()).isNotNull(); | ||
|
|
||
| final ImmutableDocument immutableDoc4 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThat(immutableDoc4.toJSON().has("secretProperty")).isTrue(); | ||
|
|
||
| final ImmutableDocument immutableDoc5 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThat(immutableDoc5.toMap()).containsKey("secretProperty"); | ||
|
|
||
| final ImmutableDocument immutableDoc6 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThat(immutableDoc6.getPropertyNames()).contains("secretProperty"); | ||
|
|
||
| } finally { | ||
| // Clean up the security listener | ||
| database.getEvents().unregisterListener(securityListener); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @Test | ||
| public void testConsistentExceptionTypesInLazyLoading() { | ||
| database.transaction(() -> { | ||
| // Create a document | ||
| final MutableDocument doc = database.newDocument("SecurityTest"); | ||
| doc.set("publicProperty", "public_value"); | ||
| doc.save(); | ||
|
|
||
| final RID documentRid = doc.getIdentity(); | ||
|
|
||
| // Test with different exception types | ||
| final AfterRecordReadListener runtimeExceptionListener = new AfterRecordReadListener() { | ||
| @Override | ||
| public Record onAfterRead(Record record) { | ||
| throw new RuntimeException("General runtime error"); | ||
| } | ||
| }; | ||
|
|
||
| final AfterRecordReadListener illegalStateListener = new AfterRecordReadListener() { | ||
| @Override | ||
| public Record onAfterRead(Record record) { | ||
| throw new IllegalStateException("Invalid state for access"); | ||
| } | ||
| }; | ||
|
|
||
| // Test with RuntimeException | ||
| database.getEvents().registerListener(runtimeExceptionListener); | ||
| try { | ||
| database.commit(); | ||
| database.begin(); | ||
|
|
||
| final ImmutableDocument immutableDoc1 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| // has() should propagate RuntimeException | ||
| assertThatThrownBy(() -> immutableDoc1.has("publicProperty")) | ||
| .isInstanceOf(RuntimeException.class) | ||
| .hasMessage("General runtime error"); | ||
|
|
||
| // get() now correctly propagates RuntimeException (FIXED) | ||
| final ImmutableDocument immutableDoc2 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc2.get("publicProperty")) | ||
| .isInstanceOf(RuntimeException.class) | ||
| .hasMessage("General runtime error"); | ||
|
|
||
| } finally { | ||
| database.getEvents().unregisterListener(runtimeExceptionListener); | ||
| } | ||
|
|
||
| // Test with IllegalStateException | ||
| database.getEvents().registerListener(illegalStateListener); | ||
| try { | ||
| database.commit(); | ||
| database.begin(); | ||
|
|
||
| final ImmutableDocument immutableDoc3 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| // has() should propagate IllegalStateException | ||
| assertThatThrownBy(() -> immutableDoc3.has("publicProperty")) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("Invalid state for access"); | ||
|
|
||
| // get() now correctly propagates IllegalStateException (FIXED) | ||
| final ImmutableDocument immutableDoc4 = (ImmutableDocument) database.lookupByRID(documentRid, false); | ||
| assertThatThrownBy(() -> immutableDoc4.get("publicProperty")) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("Invalid state for access"); | ||
|
|
||
| } finally { | ||
| database.getEvents().unregisterListener(illegalStateListener); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.