Update to structural variant data format #4057

sandertan · 2018-04-03T09:46:34Z

What? Why?

This new format will have support for detailed fusion data, with the goal of drawing fusion breakpoint events in a new fusion tab.

This PR contains 3 main commits, which were reviewed in separate PRs.

Backend changes to DB schema, importer, and validator by @sandertan, reviewed in this PR.
Backend changes to the API by @dionnezaal, reviewed and approved here: Api structural variant rebased thehyve/cbioportal#25
Display fusion tab based on genetic profiles of a study by @rnugraha, reviewed and approved here Display fusion tab based on genetic profiles of a study thehyve/cbioportal#31

RFC: RFC 31_b

Backend changes to DB schema, importer, and validator

Update database schema
Create importer for structural variants
Create validator for structural variants
Add specific validation for fusion events
Add representable test data to study_es_0
Remove old structural variant functionality (which was not used)

Column names: New, old and updated column names in all separate layers (data, database, java) can be found in this Google Sheets doc.

This is a fresh PR, after closing the slightly cluttered PR #3398.

oplantalech

I couldn't test if the test study loads, but for now there is at least something in the migration sql that you need to fix. The rest looks good.

db-scripts/src/main/resources/migration.sql

core/src/test/scripts/unit_tests_validate_data.py

pieterlukasse

Please add respective tests to TestIntegrationTest.java. @oplantalech can work on this.

core/src/test/java/org/mskcc/cbio/portal/scripts/TestImportStructralVariantData.java

pieterlukasse · 2018-05-25T09:14:25Z

service/src/main/java/org/cbioportal/service/impl/StructuralVariantServiceImpl.java

+    private StructuralVariantRepository structuralVariantRepository;
+
+    @Override
+    public List<StructuralVariant> fetchStructuralVariants(List<String> molecularProfileIds, 


would be good to have another endpoint to fetch structural variants by type, "FUSION" being one of the types. The types can be found in the EVENT_INFO column.

Please start a list of SV types. List can be 1 long for now, with only FUSION. Add a webservice that supports a type parameter and use the given type to query the EVENT_INFO column.

@rnugraha could then use this endpoint in the fusions tab.

sandertan · 2018-05-31T08:56:59Z

I've added a commit with code to test the structural variant importer and update the integration test to include structural variant data.

oplantalech

All works, great job!

oplantalech · 2019-06-07T09:03:00Z

@n1zea144 @sheridancbio @khzhu Hi guys, I have rebased this PR - it took quite some time because it has been stuck for a lot of time, more than a year! So it would be really good if you can review as soon as possible to speed the merging process up: this is the backend for the new fusion tab functionality.

sheridancbio · 2019-06-13T19:53:47Z

@oplantalech reminded me that this PR is awaiting review / merge. Apologies for letting it fall off the radar. I talked with @n1zea144 and we think we will be able to work the review of this PR into our next weekly sprint (starting June 19).

pieterlukasse · 2019-06-14T08:26:59Z

excellent! Thanks @sheridancbio!

ao508

I've left some comments and questions with some concerns I had while making a first pass on this pr.

Main points:

The SV mapper/repository beans were removed from the application context business/dao files. Was this intentional? I thought they needed to be there like the other mappers/repositorys for mybatis. Please correct me if I'm wrong though / if I'm misunderstanding.
There is only an add method in the SV DAO. Is this because we only ever expect to use the DAO to add records and the SV repository/mapper for fetching? I wonder if we might need to add a fetcher to the DAO at some point or another before we finally move away from the core package for imports. I don't think it'd hurt to have a fetcher method in the DAO but I'm also fine leaving it out if there really is no need for having it there.
CancerStudy has a hasFusionData() method. I suggested adding a check for datatype FUSION as well since the portal is still supporting that datatype.
Please consider modularizing the SV importer class to make it easier to read
I pointed out an unmerged file, probably leftover from a rebase attempt.

Other comments:
I think the validator updates and the test classes / cases looked good.

Let me know if you have any questions.

ao508 · 2019-06-19T18:26:40Z

business/src/main/resources/applicationContext-business.xml

@@ -53,14 +53,6 @@
            <property name="mutationalSignatureMapper" ref="mutationalSignatureMapper" />
        </bean>

-        <bean id="structuralVariantMapper" class="org.mybatis.spring.mapper.MapperFactoryBean">


note to self (nts): purpose of removal?

ao508 · 2019-06-19T18:28:10Z

core/src/main/java/org/mskcc/cbio/portal/dao/DaoStructuralVariant.java

+                                            "DRIVER_TIERS_FILTER",
+                                            "DRIVER_TIERS_FILTER_ANNOTATION",
+                                            };
+        bl.setFieldNames(fieldNames );


core/src/main/java/org/mskcc/cbio/portal/dao/DaoStructuralVariant.java

core/src/main/java/org/mskcc/cbio/portal/dao/MySQLbulkLoader.java

core/src/main/java/org/mskcc/cbio/portal/model/CancerStudy.java

db-scripts/src/main/resources/migration.sql

model/src/main/java/org/cbioportal/model/StructuralVariant.java

...ce/persistence-api/src/main/java/org/cbioportal/persistence/StructuralVariantRepository.java

ao508 · 2019-06-19T19:17:07Z

service/src/main/java/org/cbioportal/service/impl/StructuralVariantServiceImpl.java

+    private StructuralVariantRepository structuralVariantRepository;
+
+    @Override
+    public List<StructuralVariant> fetchStructuralVariants(List<String> molecularProfileIds, 


ao508 · 2019-06-19T19:18:24Z

web/src/main/java/org/cbioportal/web/StructuralVariantController.java

+    @Autowired
+    private StructuralVariantService structuralVariantService;
+
+    @RequestMapping(value = "/structuralvariant/fetch", method = RequestMethod.POST, 


should this also require a user permissions check?

Good catch. I have searched how is permission implemented in other endpoints and I am a bit confused: what is the difference between @PreAuthorize and @PostFilter? And why is permission sometimes implemented the service layer and in others it is in the Controller layer? Do you know it?

oplantalech · 2019-06-21T16:57:21Z

@ao508 Thank you very much for your review. I think I have answered/fixed all your issues except the removal of SV mapper/repository beans from the application context business/dao files: business is part of the old API and we have moved the files to the API layer which is part of the new API.

Now you should have all answers, feel free to reply if there are unsolved issues or otherwise approve the PR 😃

ao508

lgtm

core/src/main/java/org/mskcc/cbio/portal/dao/MySQLbulkLoader.java

jjgao · 2019-06-28T18:23:49Z

@khzhu could you create a new issue for this and assign to @oplantalech ?

khzhu · 2019-06-28T19:38:51Z

@JJ, sure. I will remove comment and open a new issue. thanks!

sandertan added feature The Hyve includes db changes labels Apr 3, 2018

sandertan assigned pieterlukasse Apr 3, 2018

sandertan requested a review from pieterlukasse April 3, 2018 09:46

oplantalech self-requested a review April 11, 2018 14:39

oplantalech requested changes Apr 12, 2018

View reviewed changes

db-scripts/src/main/resources/migration.sql Outdated Show resolved Hide resolved

db-scripts/src/main/resources/migration.sql Show resolved Hide resolved

core/src/test/scripts/unit_tests_validate_data.py Show resolved Hide resolved

pieterlukasse requested changes Apr 16, 2018

View reviewed changes

pieterlukasse reviewed Apr 16, 2018

View reviewed changes

core/src/test/java/org/mskcc/cbio/portal/scripts/TestImportStructralVariantData.java Outdated Show resolved Hide resolved

sandertan force-pushed the structural_variant_data_format branch from 8224415 to 3b37d07 Compare May 18, 2018 11:26

rnugraha force-pushed the structural_variant_data_format branch from 35ae61a to 3cc23ad Compare May 23, 2018 09:13

sandertan force-pushed the structural_variant_data_format branch from 3cc23ad to 54c6315 Compare May 24, 2018 13:38

pieterlukasse reviewed May 25, 2018

View reviewed changes

sandertan force-pushed the structural_variant_data_format branch 4 times, most recently from c029c45 to b136fa8 Compare May 25, 2018 15:15

rnugraha force-pushed the structural_variant_data_format branch from b136fa8 to 860816a Compare May 30, 2018 10:28

sandertan force-pushed the structural_variant_data_format branch from 860816a to b136fa8 Compare May 31, 2018 08:25

rnugraha force-pushed the structural_variant_data_format branch from b136fa8 to 1c26cd6 Compare May 31, 2018 08:39

sandertan assigned pieterlukasse and unassigned pieterlukasse May 31, 2018

oplantalech approved these changes Jun 4, 2018

View reviewed changes

sandertan force-pushed the structural_variant_data_format branch from b3c9a68 to 3d45344 Compare June 4, 2018 12:08

rnugraha force-pushed the structural_variant_data_format branch 4 times, most recently from 94e9241 to df68038 Compare June 18, 2018 08:49

rnugraha force-pushed the structural_variant_data_format branch from a0e9635 to c06f26a Compare June 26, 2018 10:01

oplantalech requested review from sheridancbio, khzhu and n1zea144 June 6, 2019 15:15

oplantalech force-pushed the structural_variant_data_format branch from b9ed02e to 1144243 Compare June 6, 2019 15:20

oplantalech force-pushed the structural_variant_data_format branch from b77ebbd to d05e211 Compare June 7, 2019 09:06

pieterlukasse assigned oplantalech and unassigned pieterlukasse Jun 7, 2019

oplantalech force-pushed the structural_variant_data_format branch from d05e211 to 0b8e796 Compare June 12, 2019 08:32

oplantalech force-pushed the structural_variant_data_format branch from 0b8e796 to 66d55f8 Compare June 19, 2019 12:35

ao508 requested changes Jun 19, 2019

View reviewed changes

ao508 approved these changes Jun 24, 2019

View reviewed changes

core/src/main/java/org/mskcc/cbio/portal/dao/MySQLbulkLoader.java Show resolved Hide resolved

Sander Tan and others added 7 commits June 25, 2019 09:43

Updated DB schema, importer and validator for structural variant data

3145815

API for structural variants with one POST endpoint

e2bccc4

Display fusion tab based on genetic profiles of a study

f1b7f02

Update tests and test data

034b4ad

Fix mvn tests

23d8202

Implement requested changes

79062ee

Fix tests

29c8faf

oplantalech force-pushed the structural_variant_data_format branch from d245a15 to 29c8faf Compare June 25, 2019 08:00

ao508 merged commit 8398be8 into cBioPortal:rc Jun 25, 2019

oplantalech mentioned this pull request Jun 26, 2019

Fix endpoint of molecularProfiles with structural variants #6328

Merged

jjgao mentioned this pull request Jun 28, 2019

Test on import structure variants failed #6338

Closed

inodb mentioned this pull request Jul 3, 2019

Update rc client cBioPortal/cbioportal-frontend#2522

Merged

oplantalech deleted the structural_variant_data_format branch November 22, 2022 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to structural variant data format #4057

Update to structural variant data format #4057

sandertan commented Apr 3, 2018 •

edited

Loading

oplantalech left a comment

pieterlukasse left a comment •

edited

Loading

pieterlukasse May 25, 2018 •

edited

Loading

ao508 Jun 19, 2019

sandertan commented May 31, 2018

oplantalech left a comment

oplantalech commented Jun 7, 2019 •

edited

Loading

sheridancbio commented Jun 13, 2019

pieterlukasse commented Jun 14, 2019

ao508 left a comment

ao508 Jun 19, 2019

ao508 Jun 19, 2019

ao508 Jun 19, 2019

ao508 Jun 19, 2019

oplantalech Jun 21, 2019

oplantalech commented Jun 21, 2019

ao508 left a comment

jjgao commented Jun 28, 2019

khzhu commented Jun 28, 2019

Update to structural variant data format #4057

Update to structural variant data format #4057

Conversation

sandertan commented Apr 3, 2018 • edited Loading

What? Why?

Backend changes to DB schema, importer, and validator

oplantalech left a comment

Choose a reason for hiding this comment

pieterlukasse left a comment • edited Loading

Choose a reason for hiding this comment

pieterlukasse May 25, 2018 • edited Loading

Choose a reason for hiding this comment

ao508 Jun 19, 2019

Choose a reason for hiding this comment

sandertan commented May 31, 2018

oplantalech left a comment

Choose a reason for hiding this comment

oplantalech commented Jun 7, 2019 • edited Loading

sheridancbio commented Jun 13, 2019

pieterlukasse commented Jun 14, 2019

ao508 left a comment

Choose a reason for hiding this comment

ao508 Jun 19, 2019

Choose a reason for hiding this comment

ao508 Jun 19, 2019

Choose a reason for hiding this comment

ao508 Jun 19, 2019

Choose a reason for hiding this comment

ao508 Jun 19, 2019

Choose a reason for hiding this comment

oplantalech Jun 21, 2019

Choose a reason for hiding this comment

oplantalech commented Jun 21, 2019

ao508 left a comment

Choose a reason for hiding this comment

jjgao commented Jun 28, 2019

khzhu commented Jun 28, 2019

sandertan commented Apr 3, 2018 •

edited

Loading

pieterlukasse left a comment •

edited

Loading

pieterlukasse May 25, 2018 •

edited

Loading

oplantalech commented Jun 7, 2019 •

edited

Loading