Skip to content

fix: restore E2E stability and address regressions (#11887)#11996

Open
purvanshjoshi wants to merge 9 commits intocBioPortal:masterfrom
purvanshjoshi:fix/e2e-regressions-and-stability
Open

fix: restore E2E stability and address regressions (#11887)#11996
purvanshjoshi wants to merge 9 commits intocBioPortal:masterfrom
purvanshjoshi:fix/e2e-regressions-and-stability

Conversation

@purvanshjoshi
Copy link

@purvanshjoshi purvanshjoshi commented Feb 27, 2026

[PR Description] fix: restore E2E stability and address regressions (#11887)

Fix #11887

Overview

This Pull Request restores stability to the cBioPortal End-to-End (E2E) and integration test suites. Following the closure of PR #11970, several critical stability fixes and configuration synchronizations were missing from the master branch, leading to widespread CI failures across all open PRs. This PR incorporates those missing changes and hardens the codebase against common NullPointerExceptions (NPEs) and permission logic regressions.


Proposed Changes

1. Stability & NPE Mitigation

  • NamespaceAttributeServiceImpl Logic: Added robust null guards for both outer and inner namespace attribute fetching. This prevents a critical NullPointerException that was causing deployment failures in test environments when studies or attributes were partially loaded.
  • AccessLevel Enum Expansion: Introduced the DOWNLOAD permission level to the project's security utilities to facilitate granular control over data export actions.

2. Enhanced Permission Evaluation

  • Superuser Prioritization: Refactored CancerStudyPermissionEvaluator to evaluate global superuser rights (ALL_CANCER_STUDIES) at the start of permission checks. This significantly improves performance and ensures that administrators always have consistent access across all study levels.
  • Download Groups Support: Added full support for the download_groups feature. Users can now be granted specific access to download study data independently of their read permissions, matching the intended design for restricted data sets.

3. Database & CI Synchronization

  • Schema Version Alignment: Synchronized the project db.version to 2.14.6 across the pom.xml and all core SQL scripts (cgds.sql for main/test and migration.sql). This ensures full compatibility with the schema expectations of the restoration and integration scripts used in CI.
  • GitHub Workflow Restoration: Re-introduced integration-test.yml and security-integration-test.yml to the repository's .github/workflows directory. These files are essential for providing the necessary CI visibility that was lost during previous merges.

4. Data Model & Persistence

  • CancerStudy Model Update: Added the downloadGroups property and corresponding accessors to the CancerStudy model.
  • MyBatis Mapper Integration: Updated StudyMapper.xml and SecurityMapper.xml to include the download_groups column in SQL queries for both MySQL and Clickhouse backends.

Quality Assurance & Verification

Unit Testing

Restored and synchronized critical unit test suites that were missing or out-of-date:

  • CancerStudyPermissionEvaluatorTest: Verified 100% pass rate for the new permission evaluation logic, specifically covering DOWNLOAD level access, superuser overrides, and group-based restrictions.
  • NamespaceAttributeServiceImplTest: Verified the effectiveness of the new null guards in preventing regressions during attribute fetching.

Local Build

Verified that the project compiles successfully via Maven (mvn clean compile) with all new model and security changes.


Criticality

These fixes are critical for unblocking the entire cBioPortal development pipeline. Currently, almost every open PR is failing CI due to the regressions addressed here. Merging this PR will restore a healthy state to the master branch and allow other contributions to be properly validated.


Notify Reviewers

@alisman
@inodb
@dippindots
@onursumer

Copilot AI review requested due to automatic review settings February 27, 2026 16:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to restore E2E test stability and fix regressions after PR #11970 was merged. The main objectives are to fix NullPointerExceptions in NamespaceAttributeServiceImpl, restore CancerStudyPermissionEvaluator logic for DOWNLOAD permissions, synchronize database schema version to 2.14.6, and restore missing GitHub Actions E2E workflows.

Changes:

  • Added null safety guards to NamespaceAttributeServiceImpl to prevent NullPointerExceptions when repository methods return null
  • Introduced DOWNLOAD permission level with logic in CancerStudyPermissionEvaluator to check download groups, superuser permissions, and global download group configuration
  • Updated database version from 2.14.5 to 2.14.6 in pom.xml, cgds.sql, and migration.sql
  • Restored GitHub Actions workflows for integration tests and security integration tests
  • Added downloadGroups field to CancerStudy model and corresponding MyBatis mappers
  • Added unit tests for null handling in NamespaceAttributeServiceImpl and download permission logic in CancerStudyPermissionEvaluator
  • Updated .gitignore to exclude temporary test files and agent directories

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/main/java/org/cbioportal/legacy/service/impl/NamespaceAttributeServiceImpl.java Added null guards for repository return values to prevent NullPointerException
src/test/java/org/cbioportal/legacy/service/impl/NamespaceAttributeServiceImplTest.java Added unit tests for null handling scenarios
src/main/java/org/cbioportal/legacy/utils/security/AccessLevel.java Added DOWNLOAD enum value
src/main/java/org/cbioportal/application/security/CancerStudyPermissionEvaluator.java Implemented DOWNLOAD permission logic with downloadGroups checks and superuser handling
src/test/java/org/cbioportal/application/security/CancerStudyPermissionEvaluatorTest.java Added unit tests for download permission evaluation
src/main/java/org/cbioportal/application/security/config/MethodSecurityConfig.java Added downloadGroup parameter to CancerStudyPermissionEvaluator configuration
src/main/java/org/cbioportal/legacy/model/CancerStudy.java Added downloadGroups field with getter/setter
src/main/resources/org/cbioportal/legacy/persistence/mybatis/StudyMapper.xml Added download_groups column to SELECT query
src/main/resources/org/cbioportal/legacy/persistence/mybatis/SecurityMapper.xml Added download_groups column to getCancerStudyGroups query
pom.xml Updated db.version from 2.14.5 to 2.14.6
src/main/resources/db-scripts/cgds.sql Updated DB_SCHEMA_VERSION to 2.14.6
src/main/resources/db-scripts/migration.sql Updated DB_SCHEMA_VERSION to 2.14.6
src/test/resources/cgds.sql Updated DB_SCHEMA_VERSION to 2.14.6
.github/workflows/integration-test.yml Restored E2E integration test workflow
.github/workflows/security-integration-test.yml Restored security integration test workflow
.gitignore Added patterns for temporary test files and agent directories

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

…DOWNLOAD_GROUPS column, fix SecurityMapper, upgrade CI checkout action, add 3 security unit tests
@purvanshjoshi
Copy link
Author

purvanshjoshi commented Mar 1, 2026

Cc: @inodb @alisman @dippindots

I’ve updated this PR to address the recent CI regressions and include hardening based on the Copilot review.

Notably, I've implemented a fix for the ClickHouse E2E startup crash (Column 'download_groups' not found). As @onursumer mentioned in #11886, the E2E tests are currently using a remote DB hotfix; my change in

StudyMapper.xml
ensures the backend is now forward-compatible and doesn't crash even if the underlying database schema hasn't been migrated to v2.14.6 yet.

Summary of hardening & stability fixes:

Resolved E2E Crash: Removed download_groups from StudyMapper SELECT to support the current "remote DB" CI state.
Security Hardening: Fixed a potential bypass in

CancerStudyPermissionEvaluator.java
to strictly enforce global download groups.
MyBatis Fix: Corrected the mapping mismatch in

SecurityMapper.xml.Schema Sync: Updated DDL scripts (cgds.sql and migration.sql) to v2.14.6.
Verified: All 11 unit tests for permission logic are passing.

Ready for a fresh review!"

@purvanshjoshi purvanshjoshi force-pushed the fix/e2e-regressions-and-stability branch from 367cdfb to aad6ae0 Compare March 1, 2026 16:47
@purvanshjoshi
Copy link
Author

Hi @alisman, @inodb, @dippindots, and @onursumer,

I’ve pushed updates to address the recent CI regressions and added hardening based on the Copilot review. All unit tests, security tests, and frontend builds are now passing successfully! ✅

However, the run_e2e_localdb_tests job on CircleCI is still failing. I’ve already implemented several stability fixes (such as null-guards for NamespaceAttributeServiceImpl,

AlterationEnrichmentController
, and resolving the ClickHouse download_groups startup crash), but the E2E suite is still catching something else.

Could you kindly review the current changes and provide some guidance or point me in the right direction to resolve these remaining E2E test failures? Any insights into the specific test environment setup or the failure logs would be greatly appreciated so I can get this green.

Thanks!

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.

Restore E2E GitHub Actions Tests

2 participants