[joerghoh]

Avoid unnecessary property counting:

• Node.getProperties() no longer eagerly counts properties. Consuming the full iterator without requesting its size requires one enumeration pass instead of two.
• Iterator sizes are cached until the session update counter changes, avoiding repeated scans for filtered iterators.
• Node.hasProperties() stops at the first readable property rather than counting them all.

PropertyIteratorDelegate supports filtered and unfiltered iterators through a shared lazy-size implementation.

Deferred size calculation runs inside SessionDelegate.safePerform() because the operation creating the iterator has already finished. This provides session locking and any required refresh before checking cache validity or counting.

Behavior change

Unfiltered iterator sizes were previously captured at creation. They are now calculated on demand and recalculated after session updates; the iteration itself is not rebuilt.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rishabhdaim]

🤖 AI-generated review — automated review via OpenAI Codex.

Review outcome: 2 P3 findings remain open; review follow-up required.

Human review advisory

Classification status: complete. Reviewed revision: afeb5cd550aa6b75ad65d79084cd39930af05a0d.

Checks evaluated: Snapshot: complete; Built-in policy: complete; Repository policy: absent-not-required; Review evidence: complete; Semantic assessment: reviewer-evidence.

Authority escalation: No additional owner decision identified (no_escalation_signal_detected)

Classification confidence: 87/100 (uncalibrated AI confidence; authority classification only).

Reason: Complete required evidence contains no unresolved decision requiring additional human authority.

Analysis found no additional escalation signal. This is not approval and does not replace existing review requirements.

This is an AI-generated review. Human review is required.

[rishabhdaim]

🟡 P3 · 🎯 Correctness — refresh regression coverage

🔍 Issue: The new cache is tested after local writes, but no size assertion retains a populated iterator cache across explicit or automatic reader refresh; the ACL test checks only hasProperties().

🛠️ Fix: Add these two cases to PropertyIterationTest to check the same unfiltered and filtered iterators after permission/content changes:

    /** Recounts cached sizes after an explicit refresh changes readable properties. */
    @Test
    public void cachedSizesAfterExplicitRefresh() throws Exception {
        Session reader = getAnonymousSession();
        NodeImpl<?> readable = (NodeImpl<?>) reader.getNode("/test");
        PropertyIterator[] iterators = {readable.getProperties(), readable.getProperties("tit*"),
                readable.getProperties(new String[] {"tit*"})};
        for (int i = 0; i < iterators.length; i++) {
            assertEquals(i == 0 ? 2 : 1, iterators[i].getSize());
        }

        AccessControlUtils.addAccessControlEntry(session, "/test", EveryonePrincipal.getInstance(),
                new String[] {PrivilegeConstants.REP_READ_PROPERTIES}, false);
        session.save();
        reader.refresh(false);

        for (PropertyIterator properties : iterators) {
            assertEquals(0, properties.getSize());
            assertEquals(0, properties.getSize());
            assertEquals(0, properties.getPosition());
        }
    }

    /** Recounts cached sizes when getSize performs a scheduled session refresh. */
    @Test
    public void cachedSizesAfterAutomaticRefresh() throws Exception {
        Session reader = getAnonymousSession();
        NodeImpl<?> readable = (NodeImpl<?>) reader.getNode("/test");
        PropertyIterator[] iterators = {readable.getProperties(), readable.getProperties("tit*"),
                readable.getProperties(new String[] {"tit*"})};
        for (int i = 0; i < iterators.length; i++) {
            assertEquals(i == 0 ? 2 : 1, iterators[i].getSize());
        }

        source.setProperty("title2", "value");
        session.save();
        readable.sessionDelegate.refreshAtNextAccess();

        for (int i = 0; i < iterators.length; i++) {
            assertEquals(i == 0 ? 3 : 2, iterators[i].getSize());
            assertEquals(i == 0 ? 3 : 2, iterators[i].getSize());
            assertEquals(0, iterators[i].getPosition());
        }
    }

Validated locally against afeb5cd550aa6b75ad65d79084cd39930af05a0d on Java 17 with mvn -B -pl oak-jcr -Dtest=PropertyIterationTest -Dnsfixtures=SEGMENT_TAR test: 8 tests passed, 0 failures/errors/skips, including these two added cases and the six PR tests.

[rishabhdaim]

🟡 P3 · ⚡ Performance — optional allocation improvement

🔍 Issue: Every getSize() call constructs a new SessionOperation, including calls that return the cached size.

🛠️ Fix: Reuse one operation per PropertyIteratorDelegate, keeping every invocation inside safePerform and retaining the update-count check inside perform() so refresh and synchronization still run.