fix(webauthn): use Length.LONG32 for byte[] columns to fix MariaDB row size limit

[devondragon]

Summary

Fixes #286

• Replace @Column(length = 65535) / @Column(length = 2048) with @Column(length = Length.LONG32) on the three byte[] fields in WebAuthnCredential (attestationObject, attestationClientDataJson, publicKey)
• Hibernate auto-upgrades to LONGBLOB on MariaDB/MySQL (stored off-page, avoids 65,535-byte InnoDB row size limit) and stays as bytea on PostgreSQL
• Intentionally avoids @Lob which maps to OID on PostgreSQL per Hibernate docs, causing cross-database incompatibility

Test Plan

• New WebAuthnCredentialColumnMappingTest verifies Length.LONG32 on all three byte[] fields via reflection
• Existing WebAuthnFeatureEnabledIntegrationTest passes (persists credentials with H2)
• Full test suite passes
• Verify table creation on MariaDB with ddl-auto: update (demo app)

[claude]

PR Review: fix(webauthn): use Length.LONG32 for byte[] columns

Summary

The fix is correct and well-reasoned. Using Length.LONG32 is the idiomatic Hibernate 6 approach for cross-database BLOB compatibility and avoids the @Lob / PostgreSQL OID trap. The commit message clearly explains the root cause (MariaDB InnoDB row size limit). Overall this is a solid, focused fix.

---

Correctness ✅

Length.LONG32 = Integer.MAX_VALUE = 2_147_483_647 triggers Hibernate to emit LONGBLOB on MySQL/MariaDB (stored off-page, no row-size-limit issue) and bytea on PostgreSQL. The explicit avoidance of @Lob is well-documented and correct — Hibernate 6 maps @Lob byte[] to OID on PostgreSQL, breaking cross-DB portability.

---

Import Ordering ⚠️

CLAUDE.md requires alphabetical imports. The new org.hibernate.Length import was inserted between java.time.Instant and jakarta.persistence.*:

import java.time.Instant;
import org.hibernate.Length;        // ← inserted here
import jakarta.persistence.Column;

Strictly alphabetical order would be:

import jakarta.persistence.Column;  // 'j' < 'o', and jakarta < java alphabetically
import jakarta.persistence.Entity;
// ... other jakarta imports ...
import java.time.Instant;
import lombok.Data;
import org.hibernate.Length;

The same issue exists in the test file — java.lang.reflect.Field appears after jakarta.persistence.Column, but alphabetically jakarta sorts before java.

While this is pre-existing in the file, the PR is a good opportunity to fix it.

---

Test Quality

What's good:

• Parameterized test efficiently covers all three fields in one method.
• No Spring context needed — keeps it fast and lightweight.
• The assertion message is informative, spelling out the exact constant value and the why.
• Naming follows the should[ExpectedBehavior]When[Condition] convention.

Suggestion — supplement with a schema-level test:

The reflection test verifies the annotation value exists, but it doesn't verify that Hibernate actually emits LONGBLOB (vs. BLOB or VARBINARY) for the column. A complementary @DatabaseTest using JdbcTemplate to inspect INFORMATION_SCHEMA.COLUMNS would give stronger confidence — especially valuable since the original bug was a silent schema failure. This isn't a blocker, but would close the gap:

@DatabaseTest
class WebAuthnCredentialSchemaTest {
    @Autowired JdbcTemplate jdbcTemplate;

    @Test
    void attestationObjectShouldBeLongBlob() {
        String colType = jdbcTemplate.queryForObject(
            "SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS " +
            "WHERE TABLE_NAME='user_credentials' AND COLUMN_NAME='attestation_object'",
            String.class);
        // H2 in MySQL-compat mode returns "longblob"; adjust if using default H2 mode
        assertThat(colType).isNotNull();
    }
}

(H2 in its default mode may still map this differently, so this is most valuable as a MariaDB/MySQL integration check via the demo app — which the PR already flags.)

---

Migration Concern ⚠️

Existing deployments that previously had VARBINARY(65535) or BLOB columns will need a schema migration when upgrading. Consider documenting this in MIGRATION.md (or the changelog) so library consumers know to run an ALTER TABLE user_credentials MODIFY COLUMN ... for their MariaDB/MySQL instances. With ddl-auto: update Hibernate should handle it automatically, but it's worth calling out explicitly.

---

Minor

• The @DisplayName on the test class is good; the method-level @DisplayName repeats a lot of the field name — fine as-is but "byte[] field '{0}' must use Length.LONG32 for cross-database BLOB compatibility" would read more naturally with the parameterized field name.

---

Verdict

Approve with minor suggestions. The core fix is correct. I'd recommend fixing the import order before merging (per CLAUDE.md style guidelines) and adding a note in MIGRATION.md about the schema change for existing deployments.

[Copilot]

Pull request overview

Updates the WebAuthn credential persistence mapping to avoid MariaDB/InnoDB row-size failures during Hibernate auto-DDL by forcing large byte[] columns to map as off-page BLOB types, while keeping PostgreSQL compatibility.

Changes:

• Updated WebAuthnCredential byte[] columns to use @Column(length = Length.LONG32) instead of fixed byte lengths.
• Added a unit test that asserts the @Column(length) values via reflection for the affected fields.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/main/java/com/digitalsanctuary/spring/user/persistence/model/WebAuthnCredential.java	Switches byte[] column length metadata to Length.LONG32 to influence Hibernate’s binary type selection (esp. MariaDB/MySQL).
src/test/java/com/digitalsanctuary/spring/user/persistence/model/WebAuthnCredentialColumnMappingTest.java	Adds a reflection-based regression test to ensure the annotation lengths remain Length.LONG32.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Consider adding a brief rationale to the field Javadocs (or a class-level comment) explaining that length = Length.LONG32 is intentionally used to force Hibernate to map these byte[] columns to off-page BLOB types on MariaDB/MySQL (avoiding InnoDB row-size limits) while remaining compatible with PostgreSQL. Without this context, a future “cleanup” could revert to a smaller length (e.g., 65535) and reintroduce the MariaDB DDL failure.

[Copilot]

The reflection-based assertion verifies the annotation value but not the actual DDL/type Hibernate generates for MariaDB/MySQL (the regression reported in #286). If feasible, add a test that runs Hibernate schema generation with the MariaDB dialect (or a lightweight Testcontainers MariaDB) and asserts these fields map to a BLOB/LONGBLOB type rather than VARBINARY, so the table-creation failure can’t silently return.

[claude]

Follow-up to prior review

Correction to the earlier Claude review: the Migration Concern I raised about missing MIGRATION.md documentation is actually already addressed in this PR — the diff includes a clear migration section for the user_credentials table change. Apologies for the noise.

Updated Verdict

The PR is in good shape. The only remaining actionable items are:

1. Import ordering — org.hibernate.Length and java.time.Instant appear before jakarta.persistence.* in both WebAuthnCredential.java and the test file. Per CLAUDE.md, imports should be alphabetical (jakarta sorts before java and org).

2. Test gap (non-blocking) — The reflection test guards the annotation value but not the actual DDL emitted. A schema-level check via INFORMATION_SCHEMA.COLUMNS (most useful against a real MariaDB instance via the demo app, as already noted in the PR checklist) would close the gap, but this can be a follow-up.

Everything else — the fix itself, the javadoc explaining the @Lob avoidance rationale, and the migration guide — is solid.

[claude]

Correction to prior Claude reviews: both previous Claude comments incorrectly flagged import ordering as an issue. The actual diff moves java.time.Instant to after jakarta.* and places org.hibernate.Length after lombok.Data, giving the final order jakarta -> java -> lombok -> org, which is alphabetically correct per CLAUDE.md. No import ordering issues remain. The MIGRATION.md section was also already present in the PR — both points in the second Claude review were already resolved in the diff. New tests: The PR adds AbstractSchemaValidationTest, MariaDBSchemaValidationTest, and PostgreSQLSchemaValidationTest, which directly address the schema-level coverage gap raised earlier. One concern: in AbstractSchemaValidationTest, shouldCreateWebAuthnBlobColumns queries INFORMATION_SCHEMA.COLUMNS for the user_credentials columns. If the table was never created (the exact regression being guarded against), JdbcTemplate.queryForObject throws EmptyResultDataAccessException rather than a clear assertion failure. Since JUnit 5 does not guarantee test method order, shouldCreateAllExpectedTables may not run first to give the clearer table-missing message. Suggested fix: add @TestMethodOrder(MethodOrderer.OrderAnnotation.class) on the abstract class and @order(1)/@order(2) on the two test methods, so the table-existence check always precedes the column-type check. Alternatively, catch EmptyResultDataAccessException in the column loop and fail with an explicit message. Non-blocking. Overall: the fix is correct, the imports are ordered correctly, MIGRATION.md is present, and the layered test coverage is solid. The only actionable item is improving test-ordering clarity in AbstractSchemaValidationTest before merge.