Add loadAllOfSorted to TransactionManager

[CydeWeys]

During our implementation of Expiry Access Period (XAP), adding a field to the
Registrar entity altered database heap scan order when loadAllOf() was
called without an explicit ORDER BY clause. This caused non-deterministic
row shifting in the Angular console registrar table (/console/registrars),
breaking golden image comparisons in ConsoleScreenshotTest.

To permanently fix this root cause and guarantee deterministic UI rendering
and test stability across schema evolutions, this commit introduces sorted
database loading across our persistence layer.

Specifically, this commit:

• Adds loadAllOfSorted and loadAllOfSortedStream to TransactionManager,
  JpaTransactionManagerImpl, and DelegatingReplicaJpaTransactionManager.
• Enforces strict allowlist regex validation (^[a-zA-Z0-9_.]+$) on property
  names in JpaTransactionManagerImpl before appending dynamic ORDER BY
  clauses, preventing JPQL/SQL injection since bind parameters cannot be
  used for schema identifiers.
• Adds Registrar.loadAllSorted and updates RegistrarsAction to explicitly
  sort registrars alphabetically by registrarName and registrarId.
• Updates golden screenshot
  ConsoleScreenshotTest_globalRole_registrars_registrarsPage.png to
  reflect the newly enforced deterministic alphabetical registrar ordering.
• Adds comprehensive unit test coverage in JpaTransactionManagerImplTest
  verifying exact sorted entity retrieval across multiple fields and regex
  rejection of invalid field names.

TAG=agy
CONV=f2488c74-8b4a-43f1-9c22-d1dddbdbb4e0
BUG=http://b/531889856

---

This change is 

[CydeWeys]

Add a comment explaining why this exists / what it does.

[CydeWeys]

Done. Added Javadoc explaining why regex validation is required to prevent JPQL/SQL injection in dynamic ORDER BY clauses.

[CydeWeys]

This is not necessary. You only have a few callsites in tests; it's fine to wrap them there with tm().

Also I don't think you even use this helper method anyway?!

[CydeWeys]

Done. Removed this unused helper method from DatabaseHelper.

[weiminyu]

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java line 556 at r2 (raw file):

  @Override
  public <T> Stream<T> loadAllOfSortedStream(Class<T> clazz, String... sortFields) {

We may want to have an allow/forbidden list for clazz here, just in case someone uses it on domain or host.

Code quote:

Class<T> clazz

[weiminyu]

@weiminyu reviewed 9 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on CydeWeys).

[CydeWeys]

@CydeWeys made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on weiminyu).

---

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java line 556 at r2 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

We may want to have an allow/forbidden list for clazz here, just in case someone uses it on domain or host.

loadAllOf() already exists though, and this particular footgun kind of falls into the umbrella category of "You can't really prevent people from writing bad code" (other than the normal code review process that would hopefully catch it). Someone could just as easily still call any of a number of other ways to load all entities from the database that would bypass this restriction on this one helper method, or they could even do something much more dangerous like deleting all entities of a given type.

[CydeWeys]

PTAL

@CydeWeys made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on weiminyu).

[weiminyu]

@weiminyu resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on CydeWeys).

[CydeWeys]

PTAL, had to sync to HEAD.