Skip to content

ObjectFactory fails to map reserved field names#7132

Merged
XingY merged 2 commits intodevelopfrom
fb_reservedDBField
Oct 20, 2025
Merged

ObjectFactory fails to map reserved field names#7132
XingY merged 2 commits intodevelopfrom
fb_reservedDBField

Conversation

@XingY
Copy link
Contributor

@XingY XingY commented Oct 17, 2025

Rationale

A recent change to how file fields are displayed in Sample Timeline revealed a flaw in our audit log query utils. Specifically, AuditLogService.getAuditEvents fails to populate the event beans if any of the event field name happens to be in the sql dialect's reservedWordSet. For example, "File" is a reserved word for SQL Server, but not for postgresDB. When a column is a reserved word, it's alias to "col_", "col_1"... in the select sql. The ResultSet uses "col_" as the column name, and has no knowledge of its original name, which results in bean property mapping failure.

This PR adds a workaround to allow mapping ignoring trailing "_". It also logs warning about reserved word being added to domain.

Something to consider for future work: the set of reserved word is large, and contains words like "case", "primary" that seems harmless to use as domain fields, so it's not practical to disallow them in user domains.

SMSampleTimelineBasicTest.testFileFields (MSSQL)
NPE when loading sample timeline with file field changes

Related Pull Requests

Changes

  • attempt to resolve bean field mapping by stripping trailing "_"
  • add warning about use of reserved word for domain

@XingY XingY requested a review from a team October 17, 2025 20:27
@XingY XingY self-assigned this Oct 17, 2025
@XingY XingY requested a review from labkey-matthewb October 20, 2025 17:17
}
}

if (CoreSchema.getInstance().getSqlDialect().isReserved(name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is supported, then I would not include these warnings (or lower to DEBUG). Customers will see these and ask us how to resolve the problem. If this is not fully supported...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. log removed for user defined domains.

@XingY XingY merged commit 6988f54 into develop Oct 20, 2025
7 of 10 checks passed
@XingY XingY deleted the fb_reservedDBField branch October 20, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants