Skip to content

Commit f770e0e

Browse files
committed
improvements from code review
1 parent feaecf3 commit f770e0e

26 files changed

+217
-240
lines changed

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/api/GeneExpressionApiDelegateImpl.java

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,41 +22,13 @@ public class GeneExpressionApiDelegateImpl implements GeneExpressionApiDelegate
2222

2323
@Override
2424
public ResponseEntity<GeneExpressionsPageDto> getGeneExpressions(
25-
GeneExpressionSearchQueryDto geneExpressionSearchQuery
25+
GeneExpressionSearchQueryDto query
2626
) {
27-
// Extract query parameters with defaults
28-
List<String> categories = geneExpressionSearchQuery != null
29-
? geneExpressionSearchQuery.getCategories()
30-
: null;
31-
List<String> items = geneExpressionSearchQuery != null &&
32-
geneExpressionSearchQuery.getItems() != null
33-
? geneExpressionSearchQuery.getItems()
34-
: List.of();
35-
ItemFilterTypeQueryDto filterType = geneExpressionSearchQuery != null &&
36-
geneExpressionSearchQuery.getItemFilterType() != null
37-
? geneExpressionSearchQuery.getItemFilterType()
38-
: ItemFilterTypeQueryDto.INCLUDE;
39-
Integer pageNumber = geneExpressionSearchQuery != null &&
40-
geneExpressionSearchQuery.getPageNumber() != null
41-
? geneExpressionSearchQuery.getPageNumber()
42-
: 0;
43-
Integer pageSize = geneExpressionSearchQuery != null &&
44-
geneExpressionSearchQuery.getPageSize() != null
45-
? geneExpressionSearchQuery.getPageSize()
46-
: 10;
47-
48-
String[] tissueAndSex = extractTissueAndSex(categories);
27+
String[] tissueAndSex = extractTissueAndSex(query.getCategories());
4928
String tissue = tissueAndSex[0];
5029
String sex = tissueAndSex[1];
5130

52-
GeneExpressionsPageDto results = geneExpressionService.loadGeneExpressions(
53-
tissue,
54-
sex,
55-
items,
56-
filterType,
57-
pageNumber,
58-
pageSize
59-
);
31+
GeneExpressionsPageDto results = geneExpressionService.loadGeneExpressions(query, tissue, sex);
6032

6133
return ResponseEntity.ok()
6234
.headers(ApiHelper.createNoCacheHeaders(MediaType.APPLICATION_JSON))

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/configuration/CacheConfiguration.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ public CacheManager cacheManager() {
2525
CaffeineCacheManager cacheManager = new CaffeineCacheManager();
2626

2727
// Configure cache
28-
cacheManager.setCaffeine(Caffeine.newBuilder().recordStats()); // Enable cache statistics for monitoring
28+
// Enable cache statistics for monitoring
29+
cacheManager.setCaffeine(Caffeine.newBuilder().recordStats());
2930

3031
// Define the cache names used by query services
3132
cacheManager.setCacheNames(

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/exception/DiseaseCorrelationNotFoundException.java

Lines changed: 0 additions & 15 deletions
This file was deleted.

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/exception/GeneExpressionNotFoundException.java

Lines changed: 0 additions & 19 deletions
This file was deleted.

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/exception/GlobalExceptionHandler.java

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,42 +19,6 @@
1919
@ControllerAdvice
2020
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {
2121

22-
@ExceptionHandler(ModelOverviewNotFoundException.class)
23-
protected ResponseEntity<BasicErrorDto> handleModelOverviewNotFound(
24-
ModelOverviewNotFoundException ex,
25-
NativeWebRequest request,
26-
Locale locale
27-
) {
28-
return ResponseEntity.status(HttpStatus.NOT_FOUND)
29-
.contentType(MediaType.APPLICATION_PROBLEM_JSON)
30-
.body(
31-
BasicErrorDto.builder()
32-
.title(ErrorConstants.ENTITY_NOT_FOUND.getTitle())
33-
.status(HttpStatus.NOT_FOUND.value())
34-
.detail(ex.getMessage())
35-
.instance(resolveInstance(request))
36-
.build()
37-
);
38-
}
39-
40-
@ExceptionHandler(DiseaseCorrelationNotFoundException.class)
41-
protected ResponseEntity<BasicErrorDto> handleDiseaseCorrelationNotFound(
42-
DiseaseCorrelationNotFoundException ex,
43-
NativeWebRequest request,
44-
Locale locale
45-
) {
46-
return ResponseEntity.status(HttpStatus.NOT_FOUND)
47-
.contentType(MediaType.APPLICATION_PROBLEM_JSON)
48-
.body(
49-
BasicErrorDto.builder()
50-
.title(ErrorConstants.ENTITY_NOT_FOUND.getTitle())
51-
.status(HttpStatus.NOT_FOUND.value())
52-
.detail(ex.getMessage())
53-
.instance(resolveInstance(request))
54-
.build()
55-
);
56-
}
57-
5822
@ExceptionHandler(InvalidObjectIdException.class)
5923
protected ResponseEntity<BasicErrorDto> handleInvalidObjectId(
6024
InvalidObjectIdException ex,

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/exception/ModelOverviewNotFoundException.java

Lines changed: 0 additions & 11 deletions
This file was deleted.

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/model/dto/DiseaseCorrelationIdentifier.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
@Getter
1515
public class DiseaseCorrelationIdentifier {
1616

17+
// MG-586 - Consider using existing CompositeIdentifier utility if applicable
1718
String name;
1819
String age;
1920
String sex;

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/model/dto/GeneExpressionIdentifier.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
@Getter
1515
public class GeneExpressionIdentifier {
1616

17+
// MG-586 - Consider using existing CompositeIdentifier utility if applicable
1718
String ensemblGeneId;
1819
String name;
1920

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/model/mapper/DiseaseCorrelationMapper.java

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
import java.math.BigDecimal;
44
import java.util.List;
5-
import org.sagebionetworks.model.ad.api.next.exception.DataIntegrityException;
65
import org.sagebionetworks.model.ad.api.next.model.document.DiseaseCorrelationDocument;
76
import org.sagebionetworks.model.ad.api.next.model.document.DiseaseCorrelationDocument.CorrelationResult;
87
import org.sagebionetworks.model.ad.api.next.model.dto.CorrelationResultDto;
98
import org.sagebionetworks.model.ad.api.next.model.dto.DiseaseCorrelationDto;
109
import org.sagebionetworks.model.ad.api.next.model.dto.SexDto;
10+
import org.sagebionetworks.model.ad.api.next.util.EnumConverter;
1111
import org.springframework.lang.Nullable;
1212
import org.springframework.stereotype.Component;
1313

@@ -31,7 +31,7 @@ public DiseaseCorrelationDto toDto(@Nullable DiseaseCorrelationDocument document
3131
modifiedGenes,
3232
document.getCluster(),
3333
document.getAge(),
34-
toSexDto(document.getSex())
34+
EnumConverter.toSexDto(document.getSex(), "disease correlation record")
3535
);
3636

3737
dto.setCBE(toCorrelationDto(document.getCbe()));
@@ -44,20 +44,6 @@ public DiseaseCorrelationDto toDto(@Nullable DiseaseCorrelationDocument document
4444
return dto;
4545
}
4646

47-
private SexDto toSexDto(@Nullable String value) {
48-
if (value == null) {
49-
throw new DataIntegrityException("Missing sex value in disease correlation record");
50-
}
51-
try {
52-
return SexDto.fromValue(value);
53-
} catch (IllegalArgumentException ex) {
54-
throw new DataIntegrityException(
55-
"Unexpected sex value '" + value + "' in disease correlation record",
56-
ex
57-
);
58-
}
59-
}
60-
6147
private String getCompositeId(DiseaseCorrelationDocument document) {
6248
String name = document.getName();
6349
String age = document.getAge();

apps/model-ad/api-next/src/main/java/org/sagebionetworks/model/ad/api/next/model/mapper/GeneExpressionMapper.java

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
import java.math.BigDecimal;
44
import java.util.List;
5-
import org.sagebionetworks.model.ad.api.next.exception.DataIntegrityException;
65
import org.sagebionetworks.model.ad.api.next.model.document.GeneExpressionDocument;
76
import org.sagebionetworks.model.ad.api.next.model.document.GeneExpressionDocument.FoldChangeResult;
87
import org.sagebionetworks.model.ad.api.next.model.dto.FoldChangeResultDto;
98
import org.sagebionetworks.model.ad.api.next.model.dto.GeneExpressionDto;
109
import org.sagebionetworks.model.ad.api.next.model.dto.SexCohortDto;
10+
import org.sagebionetworks.model.ad.api.next.util.EnumConverter;
1111
import org.springframework.lang.Nullable;
1212
import org.springframework.stereotype.Component;
1313

@@ -33,7 +33,7 @@ public GeneExpressionDto toDto(@Nullable GeneExpressionDocument document) {
3333
document.getModelGroup(),
3434
document.getModelType(),
3535
document.getTissue(),
36-
toSexCohortDto(document.getSex())
36+
EnumConverter.toSexCohortDto(document.getSexCohort(), "gene expression record")
3737
);
3838

3939
dto.set4months(toFoldChangeDto(document.getFourMonths()));
@@ -43,20 +43,6 @@ public GeneExpressionDto toDto(@Nullable GeneExpressionDocument document) {
4343
return dto;
4444
}
4545

46-
private SexCohortDto toSexCohortDto(@Nullable String value) {
47-
if (value == null) {
48-
throw new DataIntegrityException("Missing sex value in gene expression record");
49-
}
50-
try {
51-
return SexCohortDto.fromValue(value);
52-
} catch (IllegalArgumentException ex) {
53-
throw new DataIntegrityException(
54-
"Unexpected sex value '" + value + "' in gene expression record",
55-
ex
56-
);
57-
}
58-
}
59-
6046
private String getCompositeId(GeneExpressionDocument document) {
6147
String ensemblGeneId = document.getEnsemblGeneId();
6248
String name = document.getName();

0 commit comments

Comments
 (0)