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

Jump to bottom

Song 494 remove analysis ids #533

Merged
merged 7 commits into from
Nov 22, 2019
Merged

Song 494 remove analysis ids #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f587cf2
formatted
rtisma Nov 21, 2019
19cf9cc
removed analysisId from everywhere and tests passing
rtisma Nov 22, 2019
829a3d4
federatedIdService now depends on localIdService for analysisId gener…
rtisma Nov 22, 2019
e737907
fixed failing test
rtisma Nov 22, 2019
f178fff
formatted
rtisma Nov 22, 2019
35128ce
removed useless DUPLICATE_ANALYSIS_ATTEMPT server error
rtisma Nov 22, 2019
5e903ef
updated client and sdk
rtisma Nov 22, 2019
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
10 changes: 2 additions & 8 deletions song-client/src/main/java/bio/overture/song/client/command/ExportCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,6 @@ public class ExportCommand extends Command {
required = true)
private String outputDir;

@Parameter(
names = {INCLUDE_ANALYSIS_ID_SHORT, INCLUDE_ANALYSIS_ID_LONG},
description = "Include the analysisId field when exporting payloads",
arity = 1)
private boolean includeAnalysisId = true;

/** Dependencies */
@NonNull private SongApi songApi;

Expand Down Expand Up @@ -209,7 +203,7 @@ private void checkNumThreads() {
}

private void processStudyMode() {
val exportedPayloads = songApi.exportStudy(studyId, includeAnalysisId);
val exportedPayloads = songApi.exportStudy(studyId);
val exportStatus = exportedPayloadsToDisk("Study(" + studyId + ")", exportedPayloads);
if (exportStatus.hasErrors()) {
save(exportStatus);
Expand Down Expand Up @@ -267,7 +261,7 @@ private Status processAnalysis(String name, List<String> analysisIds) {
Status exportStatus = new Status();
List<ExportedPayload> exportedPayloads = null;
try {
exportedPayloads = songApi.exportAnalyses(analysisIds, includeAnalysisId);
exportedPayloads = songApi.exportAnalyses(analysisIds);
} catch (Throwable t) {
exportStatus.err("ExportError: %s", t.getMessage());
}
Expand Down
11 changes: 5 additions & 6 deletions song-client/src/main/java/bio/overture/song/client/command/FileUpdateCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

package bio.overture.song.client.command;

import static bio.overture.song.core.model.enums.AccessTypes.resolveAccessType;
import static java.lang.String.format;
import static java.util.Objects.isNull;

import bio.overture.song.client.config.CustomRestClientConfig;
import bio.overture.song.core.model.FileUpdateRequest;
import bio.overture.song.core.model.enums.AccessTypes;
Expand All @@ -27,16 +31,11 @@
import com.beust.jcommander.ParameterException;
import com.beust.jcommander.Parameters;
import com.fasterxml.jackson.databind.JsonNode;
import java.io.IOException;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.val;

import java.io.IOException;

import static java.lang.String.format;
import static java.util.Objects.isNull;
import static bio.overture.song.core.model.enums.AccessTypes.resolveAccessType;

@RequiredArgsConstructor
@Parameters(separators = "=", commandDescription = "Update a file")
public class FileUpdateCommand extends Command {
Expand Down
5 changes: 2 additions & 3 deletions song-client/src/test/java/bio/overture/song/client/HappyPathClientMainTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -480,8 +479,8 @@ public void testExport() {
val exportedPayload2 = createExportedPayload(DUMMY_STUDY_ID, payloads2);
val expectedResponse = List.of(exportedPayload1, exportedPayload2);

when(songApi.exportStudy(eq(DUMMY_STUDY_ID), anyBoolean())).thenReturn(expectedResponse);
when(songApi.exportAnalyses(anyList(), anyBoolean())).thenReturn(expectedResponse);
when(songApi.exportStudy(eq(DUMMY_STUDY_ID))).thenReturn(expectedResponse);
when(songApi.exportAnalyses(anyList())).thenReturn(expectedResponse);

val outputDir = tmp.newFolder().toPath();
val studyDir = outputDir.resolve(DUMMY_STUDY_ID);
Expand Down
1 change: 0 additions & 1 deletion song-core/src/main/java/bio/overture/song/core/exceptions/ServerErrors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public enum ServerErrors implements ServerError {
SPECIMEN_ALREADY_EXISTS(CONFLICT),
VARIANT_CALL_CORRUPTED_DUPLICATE(INTERNAL_SERVER_ERROR),
SEQUENCING_READ_CORRUPTED_DUPLICATE(INTERNAL_SERVER_ERROR),
DUPLICATE_ANALYSIS_ATTEMPT(CONFLICT),
STUDY_ALREADY_EXISTS(CONFLICT),
MISMATCHING_STORAGE_OBJECT_SIZES(CONFLICT),
MISMATCHING_STORAGE_OBJECT_CHECKSUMS(CONFLICT),
Expand Down
16 changes: 7 additions & 9 deletions song-java-sdk/src/main/java/bio/overture/song/sdk/SongApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package bio.overture.song.sdk;

import static java.lang.Boolean.parseBoolean;

import bio.overture.song.core.model.Analysis;
import bio.overture.song.core.model.AnalysisType;
import bio.overture.song.core.model.ExportedPayload;
Expand All @@ -27,16 +29,13 @@
import bio.overture.song.sdk.model.ListAnalysisTypesRequest;
import bio.overture.song.sdk.web.Endpoint;
import bio.overture.song.sdk.web.RestClient;
import java.util.List;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.SneakyThrows;
import lombok.val;
import org.springframework.web.client.ResourceAccessException;

import java.util.List;

import static java.lang.Boolean.parseBoolean;

@RequiredArgsConstructor
public class SongApi {

Expand Down Expand Up @@ -105,14 +104,13 @@ public String unpublish(@NonNull String studyId, @NonNull String analysisId) {
return restClient.put(url, String.class).getBody();
}

public List<ExportedPayload> exportStudy(@NonNull String studyId, boolean includeAnalysisId) {
val url = endpoint.exportStudy(studyId, includeAnalysisId);
public List<ExportedPayload> exportStudy(@NonNull String studyId) {
val url = endpoint.exportStudy(studyId);
return restClient.getList(url, ExportedPayload.class).getBody();
}

public List<ExportedPayload> exportAnalyses(
@NonNull List<String> analysisIds, boolean includeAnalysisId) {
val url = endpoint.exportAnalysisIds(analysisIds, includeAnalysisId);
public List<ExportedPayload> exportAnalyses(@NonNull List<String> analysisIds) {
val url = endpoint.exportAnalysisIds(analysisIds);
return restClient.getList(url, ExportedPayload.class).getBody();
}

Expand Down
11 changes: 4 additions & 7 deletions song-java-sdk/src/main/java/bio/overture/song/sdk/web/Endpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,12 @@ public String updateFile(@NonNull String studyId, @NonNull String objectId) {
return format("%s/studies/%s/files/%s", serverUrl, studyId, objectId);
}

public String exportAnalysisIds(@NonNull List<String> analysisIds, boolean includeAnalysisId) {
return format(
"%s/export/analysis/%s?includeAnalysisId=%s",
serverUrl, COMMA.join(analysisIds), includeAnalysisId);
public String exportAnalysisIds(@NonNull List<String> analysisIds) {
return format("%s/export/analysis/%s", serverUrl, COMMA.join(analysisIds));
}

public String exportStudy(@NonNull String studyId, boolean includeAnalysisId) {
return format(
"%s/export/studies/%s?includeAnalysisId=%s", serverUrl, studyId, includeAnalysisId);
public String exportStudy(@NonNull String studyId) {
return format("%s/export/studies/%s", serverUrl, studyId);
}

public String suppress(@NonNull String studyId, @NonNull String analysisId) {
Expand Down
4 changes: 0 additions & 4 deletions song-java-sdk/src/main/java/bio/overture/song/sdk/web/RestClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@

import static bio.overture.song.core.utils.Deserialization.deserializeList;
import static bio.overture.song.core.utils.Deserialization.deserializePage;
import static bio.overture.song.core.utils.JsonUtils.mapper;
import static org.springframework.http.ResponseEntity.status;

import bio.overture.song.core.exceptions.ServerException;
import bio.overture.song.core.model.PageDTO;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.List;
import lombok.NonNull;
import lombok.SneakyThrows;
Expand All @@ -33,8 +31,6 @@

public interface RestClient {

ObjectMapper MAPPER = mapper();

<R> ResponseEntity<R> get(String endpoint, Class<R> responseType) throws ServerException;

<R> ResponseEntity<R> post(String endpoint, Object body, Class<R> responseType)
Expand Down
20 changes: 5 additions & 15 deletions song-server/src/main/java/bio/overture/song/server/config/IdConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import bio.overture.song.server.properties.IdProperties;
import bio.overture.song.server.properties.IdProperties.FederatedProperties.AuthProperties.BearerProperties;
import bio.overture.song.server.repository.AnalysisRepository;
import bio.overture.song.server.service.auth.StaticTokenService;
import bio.overture.song.server.service.id.FederatedIdService;
import bio.overture.song.server.service.id.IdService;
Expand Down Expand Up @@ -56,35 +55,26 @@ public class IdConfig {
/** Dependencies */
private final IdProperties idProperties;

private final AnalysisRepository analysisRepository;
private final RetryTemplate retryTemplate;

@Autowired
public IdConfig(
@NonNull IdProperties idProperties,
@NonNull AnalysisRepository analysisRepository,
@NonNull RetryTemplate retryTemplate) {
public IdConfig(@NonNull IdProperties idProperties, @NonNull RetryTemplate retryTemplate) {
this.idProperties = idProperties;
this.analysisRepository = analysisRepository;
this.retryTemplate = retryTemplate;
}

@Bean
public NameBasedGenerator nameBasedGenerator() {
return createNameBasedGenerator();
}

@Bean
public IdService idService(@Autowired NameBasedGenerator nameBasedGenerator) {
public IdService idService() {
val localIdService = new LocalIdService(createNameBasedGenerator());
if (idProperties.isUseLocal()) {
log.info("Loading LOCAL mode for IdService");
return new LocalIdService(nameBasedGenerator, analysisRepository);
return localIdService;
} else {
log.info("Loading FEDERATED mode for IdService");
val uriResolver = createUriResolver(idProperties.getFederated().getUriTemplate());
val restTemplate = restTemplate(idProperties.getFederated().getAuth().getBearer());
val restClient = new RestClient(restTemplate, retryTemplate);
return new FederatedIdService(restClient, uriResolver);
return new FederatedIdService(restClient, localIdService, uriResolver);
}
}

Expand Down
12 changes: 4 additions & 8 deletions song-server/src/main/java/bio/overture/song/server/controller/ExportController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.RestController;

Expand All @@ -43,10 +42,8 @@ public class ExportController {
@ApiOperation(value = "ExportStudy", notes = "Exports all the payloads for a study")
@GetMapping(value = "/studies/{studyId}")
@ResponseBody
public List<ExportedPayload> exportStudy(
@PathVariable("studyId") String studyId,
@RequestParam(value = "includeAnalysisId", defaultValue = "true") boolean includeAnalysisId) {
return exportService.exportPayloadsForStudy(studyId, includeAnalysisId);
public List<ExportedPayload> exportStudy(@PathVariable("studyId") String studyId) {
return exportService.exportPayloadsForStudy(studyId);
}

@ApiOperation(value = "ExportAnalysis", notes = "Exports the payload for a list of analysisIds")
Expand All @@ -55,8 +52,7 @@ public List<ExportedPayload> exportStudy(
public List<ExportedPayload> exportAnalysis(
@PathVariable("analysisIds")
@ApiParam(value = "Comma separated list of analysisIds", required = true)
List<String> analysisIds,
@RequestParam(value = "includeAnalysisId", defaultValue = "true") boolean includeAnalysisId) {
return exportService.exportPayload(analysisIds, includeAnalysisId);
List<String> analysisIds) {
return exportService.exportPayload(analysisIds);
}
}
16 changes: 1 addition & 15 deletions song-server/src/main/java/bio/overture/song/server/controller/UploadController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,6 @@ public SubmitResponse submit(
@RequestHeader(value = AUTHORIZATION, required = false) final String accessToken,
@PathVariable("studyId") String studyId,
@RequestBody @Valid String json_payload) {
return uploadService.submit(studyId, json_payload, false);
}

@ApiOperation(
value = "ForceSubmit",
notes = "Forcefully submit a json payload, ignoring analysisId collisions")
@PostMapping(
value = "/{studyId}/force",
consumes = {APPLICATION_JSON_VALUE, APPLICATION_JSON_UTF8_VALUE})
@PreAuthorize("@systemSecurity.authorize(authentication)")
public SubmitResponse forceSubmit(
@RequestHeader(value = AUTHORIZATION, required = false) final String accessToken,
@PathVariable("studyId") String studyId,
@RequestBody @Valid String json_payload) {
return uploadService.submit(studyId, json_payload, true);
return uploadService.submit(studyId, json_payload);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should check to make sure the client code and sdk examples match the server changes.

Specifically, I noticed song-python-sdk/examples/example_upload.py, line 76 still has "ignore_analysis_id_collisions" as a parameter to api.save.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you for the reminder. I completely forgot lol

}
}
3 changes: 3 additions & 0 deletions song-server/src/main/java/bio/overture/song/server/converter/FileConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import static org.mapstruct.factory.Mappers.getMapper;

import bio.overture.song.core.model.FileDTO;
import bio.overture.song.core.model.FileData;
import bio.overture.song.core.model.FileUpdateRequest;
import bio.overture.song.server.config.ConverterConfig;
Expand Down Expand Up @@ -62,6 +63,8 @@ default FileEntity copyFile(FileEntity ref) {

List<FileEntity> copyFiles(List<FileEntity> files);

FileDTO convertToFileDTO(FileEntity f);

default StorageObject toStorageObject(FileEntity file) {
return StorageObject.builder()
.objectId(file.getObjectId())
Expand Down
1 change: 0 additions & 1 deletion song-server/src/main/java/bio/overture/song/server/model/dto/Payload.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
public class Payload extends DynamicData {

private String study;
private String analysisId;
private AnalysisTypeId analysisType;
private List<CompositeEntity> sample;
private List<FileEntity> file;
Expand Down
9 changes: 0 additions & 9 deletions song-server/src/main/java/bio/overture/song/server/properties/IdProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,6 @@ public static class UriTemplateProperties {
private String specimen;
private String sample;
private String file;
private final AnalysisTemplateProperties analysis = new AnalysisTemplateProperties();

@Getter
@Setter
public static class AnalysisTemplateProperties {
private String existence;
private String generate;
private String save;
}
}

@Getter
Expand Down
Loading