fix: make extension_version.published_by_id NOT NULL by cstamas · Pull Request #2276 · eclipse-openvsx/openvsx · GitHub
Skip to content

fix: make extension_version.published_by_id NOT NULL - #2276

Merged
netomi merged 3 commits into
mainfrom
cstamas/issue-2266
Sep 30, 2026
Merged

netomi merged 3 commits into
mainfrom
cstamas/issue-2266

Conversation

@cstamas

@cstamas cstamas commented Sep 30, 2026

Copy link
Copy Markdown
Member

Changes

  • added migration; just sets NOT NULL constraint as there should be no NULL values
  • modified ExtensionVersion: @manytoone on publishedBy becomes @manytoone(optional = false) plus @joincolumn(name = "published_by_id", nullable = false)
  • left in places null checks in rest of the places; they are no redundant, null is enforced at DB level
  • fixed tests that now failed (did not set user on entity)
  • added one test that ensures that persisting with null user fails

fixes: #2266

Changes
* added migration; just sets NOT NULL constraint as there should be no NULL values
* modified ExtensionVersion: @manytoone on publishedBy becomes @manytoone(optional = false) plus @joincolumn(name = "published_by_id", nullable = false)
* left in places null checks in rest of the places; they are no redundant, null is enforced at DB level
* fixed tests that now failed (did not set user on entity)
* added one test that ensures that persisting with null user fails

fixes: #2266
@cstamas
cstamas requested a review from netomi September 30, 2026 10:06
@cstamas cstamas added the bug Something is not working correctly label Sep 30, 2026
@cstamas
cstamas marked this pull request as ready for review September 30, 2026 10:53
@netomi
netomi requested a balanced review from Copilot September 30, 2026 11:16
@netomi

netomi commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Generated jOOQ metadata remains stale, and several new publisher fixtures are not cleaned from the shared test database.

Review effort: Balanced
Findings: 4 Medium severity

Open (4)
What changed in this PR

Enforces non-null publishers for extension versions, resolving #2266.

Changes:

  • Adds the database constraint and matching JPA mapping.
  • Updates affected test fixtures.
  • Adds coverage for rejecting versions without publishers.
File Description
ExtensionVersion.java Marks publishedBy as required.
V1_78__Published_By_Not_Null.sql Adds the NOT NULL constraint.
ExtensionVersionChangeRepositoryTest.java Adds publisher validation coverage.
RepositoryServiceSmokeTest.java Supplies a publisher fixture.
AwsLogIngestionHandlerTest.java Supplies a publisher fixture.
DownloadIngestionProcessorTest.java Supplies a publisher fixture.
DownloadAnalyticsEndpointTest.java Supplies a publisher fixture.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The migration should avoid scanning the full table while holding an exclusive lock.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (3)
Files not reviewed (1)
  • server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionVersion.java: Generated file

Validate a NOT VALID check constraint under a weaker lock first, so the
table scan doesn't run while SET NOT NULL holds the exclusive lock on
extension_version.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The database, ORM, generated metadata, and tests consistently enforce the new invariant.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Files not reviewed (1)
  • server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/ExtensionVersion.java: Generated file

@netomi
netomi merged commit 53d09b0 into main Sep 30, 2026
6 checks passed
@netomi
netomi deleted the cstamas/issue-2266 branch September 30, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is not working correctly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extension_version.published_by_id should be NOT NULL

3 participants