Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contribution to project quality by mitigating code smells. #11240

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Contribution to project quality by mitigating code smells. #11240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fbe2648
Resolving code smells of large class and long method using extract cl…
Vinif145 Nov 28, 2024
28484d4
New version of CustomEhcachingProvider.java
Vinif145 Nov 28, 2024
70701fa
Fix of large class code smell on the class SessionServiceController.j…
Vinif145 Nov 28, 2024
6a15134
Fix of long method code smell on the class DataBinner.java who belong…
Vinif145 Nov 28, 2024
48153a4
Fix of feature envy code smell on the class AlterationMyBasisReposito…
Lucashokama Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 38 additions & 157 deletions src/main/java/org/cbioportal/persistence/mybatis/AlterationMyBatisRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
package org.cbioportal.persistence.mybatis;

import org.cbioportal.model.AlterationCountByGene;
import org.cbioportal.model.AlterationCountByStructuralVariant;
import org.cbioportal.model.AlterationFilter;
import org.cbioportal.model.CNA;
import org.cbioportal.model.CopyNumberCountByGene;
import org.cbioportal.model.MolecularProfile;
import org.cbioportal.model.MolecularProfileCaseIdentifier;
import org.cbioportal.model.MutationEventType;
import org.cbioportal.model.MolecularProfile.MolecularAlterationType;
import org.cbioportal.model.util.Select;
import org.cbioportal.model.*;
import org.cbioportal.persistence.AlterationRepository;
import org.cbioportal.persistence.MolecularProfileRepository;
import org.cbioportal.persistence.util.MolecularProfileUtil;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Repository;

Expand All @@ -23,40 +14,28 @@ public class AlterationMyBatisRepository implements AlterationRepository {

@Autowired
private AlterationCountsMapper alterationCountsMapper;

@Autowired
private MolecularProfileRepository molecularProfileRepository;
private MolecularProfileUtil molecularProfileUtil; // Dependency injection of the newly created MolecularProfileUtil for handling molecular profile-specific logic.

@Override
public List<AlterationCountByGene> getSampleAlterationGeneCounts(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers,
Select<Integer> entrezGeneIds,
AlterationFilter alterationFilter) {

if ((alterationFilter.getMutationTypeSelect().hasNone() && alterationFilter.getCNAEventTypeSelect().hasNone()
&& !alterationFilter.getStructuralVariants())
|| (molecularProfileCaseIdentifiers == null || molecularProfileCaseIdentifiers.isEmpty())
|| allAlterationsExcludedDriverAnnotation(alterationFilter)
|| allAlterationsExcludedMutationStatus(alterationFilter)
|| allAlterationsExcludedDriverTierAnnotation(alterationFilter)
) {
// We want a mutable empty list:
return new ArrayList<>();
if (filtrosInvalidos(molecularProfileCaseIdentifiers, alterationFilter)) {
return Collections.emptyList();
}

Set<String> molecularProfileIds = molecularProfileCaseIdentifiers.stream()
.map(MolecularProfileCaseIdentifier::getMolecularProfileId)
.collect(Collectors.toSet());
Map<String, MolecularAlterationType> profileTypeByProfileId = molecularProfileRepository
.getMolecularProfiles(molecularProfileIds, "SUMMARY")
.stream()
.collect(Collectors.toMap(datum -> datum.getMolecularProfileId().toString(), MolecularProfile::getMolecularAlterationType));
Map<MolecularAlterationType, List<MolecularProfileCaseIdentifier>> groupedIdentifiersByProfileType =
alterationCountsMapper.getMolecularProfileCaseInternalIdentifier(new ArrayList<>(molecularProfileCaseIdentifiers), "SAMPLE_ID")
.stream()
.collect(Collectors.groupingBy(e -> profileTypeByProfileId.getOrDefault(e.getMolecularProfileId(), null)));
// Grouping molecular profile case identifiers by alteration type has been moved to MolecularProfileUtil.
Map<MolecularProfile.MolecularAlterationType, List<MolecularProfileCaseIdentifier>> groupedIdentifiersByProfileType =
molecularProfileUtil.groupIdentifiersByProfileType(molecularProfileCaseIdentifiers, "SAMPLE_ID");

return alterationCountsMapper.getSampleAlterationGeneCounts(
groupedIdentifiersByProfileType.get(MolecularAlterationType.MUTATION_EXTENDED),
groupedIdentifiersByProfileType.get(MolecularAlterationType.COPY_NUMBER_ALTERATION),
groupedIdentifiersByProfileType.get(MolecularAlterationType.STRUCTURAL_VARIANT),
// The grouped identifiers are now fetched directly from the utility class, which centralizes this logic.
groupedIdentifiersByProfileType.get(MolecularProfile.MolecularAlterationType.MUTATION_EXTENDED),
groupedIdentifiersByProfileType.get(MolecularProfile.MolecularAlterationType.COPY_NUMBER_ALTERATION),
groupedIdentifiersByProfileType.get(MolecularProfile.MolecularAlterationType.STRUCTURAL_VARIANT),
entrezGeneIds,
createMutationTypeList(alterationFilter),
createCnaTypeList(alterationFilter),
Expand All @@ -67,42 +46,27 @@ public List<AlterationCountByGene> getSampleAlterationGeneCounts(Set<MolecularPr
alterationFilter.getIncludeUnknownTier(),
alterationFilter.getIncludeGermline(),
alterationFilter.getIncludeSomatic(),
alterationFilter.getIncludeUnknownStatus());
alterationFilter.getIncludeUnknownStatus()
);
}

@Override
public List<AlterationCountByGene> getPatientAlterationGeneCounts(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers,
Select<Integer> entrezGeneIds,
AlterationFilter alterationFilter) {

if ((alterationFilter.getMutationTypeSelect().hasNone() && alterationFilter.getCNAEventTypeSelect().hasNone()
&& !alterationFilter.getStructuralVariants())
|| (molecularProfileCaseIdentifiers == null || molecularProfileCaseIdentifiers.isEmpty())
|| allAlterationsExcludedDriverAnnotation(alterationFilter)
|| allAlterationsExcludedMutationStatus(alterationFilter)
|| allAlterationsExcludedDriverTierAnnotation(alterationFilter)) {
if (filtrosInvalidos(molecularProfileCaseIdentifiers, alterationFilter)) {
return Collections.emptyList();
}

Set<String> molecularProfileIds = molecularProfileCaseIdentifiers.stream()
.map(MolecularProfileCaseIdentifier::getMolecularProfileId)
.collect(Collectors.toSet());

Map<String, MolecularAlterationType> profileTypeByProfileId = molecularProfileRepository
.getMolecularProfiles(molecularProfileIds, "SUMMARY")
.stream()
.collect(Collectors.toMap(datum -> datum.getMolecularProfileId().toString(), MolecularProfile::getMolecularAlterationType));

Map<MolecularAlterationType, List<MolecularProfileCaseIdentifier>> groupedIdentifiersByProfileType =
alterationCountsMapper.getMolecularProfileCaseInternalIdentifier(new ArrayList<>(molecularProfileCaseIdentifiers), "PATIENT_ID")
.stream()
.collect(Collectors.groupingBy(e -> profileTypeByProfileId.getOrDefault(e.getMolecularProfileId(), null)));

// Similar to the sample counts method, the grouping logic is now abstracted in MolecularProfileUtil
Map<MolecularProfile.MolecularAlterationType, List<MolecularProfileCaseIdentifier>> groupedIdentifiersByProfileType =
molecularProfileUtil.groupIdentifiersByProfileType(molecularProfileCaseIdentifiers, "PATIENT_ID");

return alterationCountsMapper.getPatientAlterationGeneCounts(
groupedIdentifiersByProfileType.get(MolecularAlterationType.MUTATION_EXTENDED),
groupedIdentifiersByProfileType.get(MolecularAlterationType.COPY_NUMBER_ALTERATION),
groupedIdentifiersByProfileType.get(MolecularAlterationType.STRUCTURAL_VARIANT),
groupedIdentifiersByProfileType.get(MolecularProfile.MolecularAlterationType.MUTATION_EXTENDED),
groupedIdentifiersByProfileType.get(MolecularProfile.MolecularAlterationType.COPY_NUMBER_ALTERATION),
groupedIdentifiersByProfileType.get(MolecularProfile.MolecularAlterationType.STRUCTURAL_VARIANT),
entrezGeneIds,
createMutationTypeList(alterationFilter),
createCnaTypeList(alterationFilter),
Expand All @@ -117,115 +81,34 @@ public List<AlterationCountByGene> getPatientAlterationGeneCounts(Set<MolecularP
);
}

@Override
public List<CopyNumberCountByGene> getSampleCnaGeneCounts(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers,
Select<Integer> entrezGeneIds,
AlterationFilter alterationFilter) {

if (alterationFilter.getCNAEventTypeSelect().hasNone() || molecularProfileCaseIdentifiers == null
|| allAlterationsExcludedDriverAnnotation(alterationFilter)
|| allAlterationsExcludedDriverTierAnnotation(alterationFilter)) {
return Collections.emptyList();
}

List<MolecularProfileCaseIdentifier> molecularProfileCaseInternalIdentifiers =
alterationCountsMapper.getMolecularProfileCaseInternalIdentifier(new ArrayList<>(molecularProfileCaseIdentifiers), "SAMPLE_ID");

return alterationCountsMapper.getSampleCnaGeneCounts(
molecularProfileCaseInternalIdentifiers,
entrezGeneIds,
createCnaTypeList(alterationFilter),
alterationFilter.getIncludeDriver(),
alterationFilter.getIncludeVUS(),
alterationFilter.getIncludeUnknownOncogenicity(),
alterationFilter.getSelectedTiers(),
alterationFilter.getIncludeUnknownTier());
}

@Override
public List<CopyNumberCountByGene> getPatientCnaGeneCounts(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers,
Select<Integer> entrezGeneIds,
AlterationFilter alterationFilter) {

if (alterationFilter.getCNAEventTypeSelect().hasNone() || molecularProfileCaseIdentifiers == null
private boolean filtrosInvalidos(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers, AlterationFilter alterationFilter) {
return (alterationFilter.getMutationTypeSelect().hasNone() && alterationFilter.getCNAEventTypeSelect().hasNone()
&& !alterationFilter.getStructuralVariants())
|| molecularProfileCaseIdentifiers == null || molecularProfileCaseIdentifiers.isEmpty()
|| allAlterationsExcludedDriverAnnotation(alterationFilter)
|| allAlterationsExcludedDriverTierAnnotation(alterationFilter)) {
return Collections.emptyList();
}
List<MolecularProfileCaseIdentifier> molecularProfileCaseInternalIdentifiers =
alterationCountsMapper.getMolecularProfileCaseInternalIdentifier(new ArrayList<>(molecularProfileCaseIdentifiers), "PATIENT_ID");

return alterationCountsMapper.getPatientCnaGeneCounts(
molecularProfileCaseInternalIdentifiers,
entrezGeneIds,
createCnaTypeList(alterationFilter),
alterationFilter.getIncludeDriver(),
alterationFilter.getIncludeVUS(),
alterationFilter.getIncludeUnknownOncogenicity(),
alterationFilter.getSelectedTiers(),
alterationFilter.getIncludeUnknownTier());
}

@Override
public List<AlterationCountByStructuralVariant> getSampleStructuralVariantCounts(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers,
AlterationFilter alterationFilter) {

if (molecularProfileCaseIdentifiers == null
|| molecularProfileCaseIdentifiers.isEmpty()
|| allAlterationsExcludedMutationStatus(alterationFilter)) {
return Collections.emptyList();
}

return alterationCountsMapper.getSampleStructuralVariantCounts(
new ArrayList<>(molecularProfileCaseIdentifiers),
alterationFilter.getIncludeDriver(),
alterationFilter.getIncludeVUS(),
alterationFilter.getIncludeUnknownOncogenicity(),
alterationFilter.getSelectedTiers(),
alterationFilter.getIncludeUnknownTier(),
alterationFilter.getIncludeGermline(),
alterationFilter.getIncludeSomatic(),
alterationFilter.getIncludeUnknownStatus());
|| allAlterationsExcludedMutationStatus(alterationFilter)
|| allAlterationsExcludedDriverTierAnnotation(alterationFilter);
}

@Override
public List<AlterationCountByStructuralVariant> getPatientStructuralVariantCounts(Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers,
AlterationFilter alterationFilter) {

if (molecularProfileCaseIdentifiers == null
|| molecularProfileCaseIdentifiers.isEmpty()
|| allAlterationsExcludedMutationStatus(alterationFilter)) {
return Collections.emptyList();
}

return alterationCountsMapper.getPatientStructuralVariantCounts(
new ArrayList<>(molecularProfileCaseIdentifiers),
alterationFilter.getIncludeDriver(),
alterationFilter.getIncludeVUS(),
alterationFilter.getIncludeUnknownOncogenicity(),
alterationFilter.getSelectedTiers(),
alterationFilter.getIncludeUnknownTier(),
alterationFilter.getIncludeGermline(),
alterationFilter.getIncludeSomatic(),
alterationFilter.getIncludeUnknownStatus());
}

private Select<Short> createCnaTypeList(final AlterationFilter alterationFilter) {
if (alterationFilter.getCNAEventTypeSelect().hasNone())
if (alterationFilter.getCNAEventTypeSelect().hasNone()) {
return Select.none();
if (alterationFilter.getCNAEventTypeSelect().hasAll())
}
if (alterationFilter.getCNAEventTypeSelect().hasAll()) {
return Select.all();
}
return alterationFilter.getCNAEventTypeSelect().map(CNA::getCode);
}

private Select<String> createMutationTypeList(final AlterationFilter alterationFilter) {
if (alterationFilter.getMutationTypeSelect().hasNone())
if (alterationFilter.getMutationTypeSelect().hasNone()) {
return Select.none();
if (alterationFilter.getMutationTypeSelect().hasAll())
}
if (alterationFilter.getMutationTypeSelect().hasAll()) {
return Select.all();
}
Select<String> mappedMutationTypes = alterationFilter.getMutationTypeSelect().map(MutationEventType::getMutationType);
mappedMutationTypes.inverse(alterationFilter.getMutationTypeSelect().inverse());

return mappedMutationTypes;
}

Expand All @@ -239,8 +122,6 @@ private boolean allAlterationsExcludedDriverAnnotation(AlterationFilter alterati
}

private boolean allAlterationsExcludedDriverTierAnnotation(AlterationFilter alterationFilter) {
return alterationFilter.getSelectedTiers().hasNone()
&& !alterationFilter.getIncludeUnknownTier();
return alterationFilter.getSelectedTiers().hasNone() && !alterationFilter.getIncludeUnknownTier();
}

}
42 changes: 42 additions & 0 deletions src/main/java/org/cbioportal/persistence/mybatis/util/MolecularProfileUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.cbioportal.persistence.util;

import org.cbioportal.model.MolecularProfile;
import org.cbioportal.model.MolecularProfileCaseIdentifier;
import org.cbioportal.persistence.MolecularProfileRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import java.util.*;
import java.util.stream.Collectors;

@Component
public class MolecularProfileUtil {

@Autowired
private MolecularProfileRepository molecularProfileRepository;

public Map<MolecularProfile.MolecularAlterationType, List<MolecularProfileCaseIdentifier>> groupIdentifiersByProfileType(
Set<MolecularProfileCaseIdentifier> molecularProfileCaseIdentifiers, String caseIdentifierType) {

if (molecularProfileCaseIdentifiers == null || molecularProfileCaseIdentifiers.isEmpty()) {
return Collections.emptyMap();
}

Set<String> molecularProfileIds = molecularProfileCaseIdentifiers.stream()
.map(MolecularProfileCaseIdentifier::getMolecularProfileId)
.collect(Collectors.toSet());

Map<String, MolecularProfile.MolecularAlterationType> profileTypeByProfileId = molecularProfileRepository
.getMolecularProfiles(molecularProfileIds, "SUMMARY")
.stream()
.collect(Collectors.toMap(
molecularProfile -> molecularProfile.getMolecularProfileId().toString(),
MolecularProfile::getMolecularAlterationType
));

return molecularProfileCaseIdentifiers.stream()
.collect(Collectors.groupingBy(
identifier -> profileTypeByProfileId.getOrDefault(identifier.getMolecularProfileId(), null)
));
}
}
Loading