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

Remove dcc common and dcc id dependency#503 #574

Merged
merged 5 commits into from
Jan 8, 2020
Merged

Remove dcc common and dcc id dependency#503 #574

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0312dae
Enhancement: No longer includes dcc-common/dcc-id modules. Compiles.
KevinFHartmann Jan 6, 2020
5923936
Bugfix: Now compiles and runs unit tests (mvn clean package runs)
KevinFHartmann Jan 6, 2020
2ae28f4
Bugfix: Now runs all unit-test and make test-* steps correctly
KevinFHartmann Jan 7, 2020
dd5fab6
Merge branch 'develop' into Remove_dcc-common_and_dcc-id_dependency#503
KevinFHartmann Jan 7, 2020
d42e264
Change: Now use a Separators enum consisting of Joiners and Splitters…
KevinFHartmann Jan 7, 2020
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
27 changes: 2 additions & 25 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@
<id>jitpack.io</id>
<url>https://jitpack.io</url>
</repository>
<repository>
<id>dcc-releases</id>
<url>https://artifacts.oicr.on.ca/artifactory/dcc-release</url>
</repository>
<repository>
<id>dcc-snapshot</id>
<url>https://artifacts.oicr.on.ca/artifactory/dcc-snapshot</url>
</repository>
<repository>
<id>Atlassian 3rd Party</id>
<url>https://maven.atlassian.com/3rdparty/</url>
Expand Down Expand Up @@ -243,9 +235,8 @@
<version>${java-assist.version}</version>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
<version>${jaxb.version}</version>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
</dependency>

<dependency>
Expand All @@ -254,18 +245,6 @@
<version>${javax.interceptor-api.version}</version>
</dependency>

<!--ICGC Libs-->
<dependency>
<groupId>org.icgc.dcc</groupId>
<artifactId>dcc-common-core</artifactId>
<version>${dcc-common.version}</version>
</dependency>
<dependency>
<groupId>org.icgc.dcc</groupId>
<artifactId>dcc-id-client</artifactId>
<version>${dcc-id.version}</version>
</dependency>

<!--Json Schema-->
<dependency>
<groupId>com.networknt</groupId>
Expand Down Expand Up @@ -474,7 +453,6 @@
<!--<spring-data-commons.version>1.12.11.RELEASE</spring-data-commons.version>-->
<spring-framework.version>5.1.9.RELEASE</spring-framework.version>
<spring-cloud.version>Greenwich.SR3</spring-cloud.version>
<dcc-common.version>4.3.8</dcc-common.version>
<postgresql.version>42.2.2</postgresql.version>
<junit.version>4.12</junit.version>
<slf4j.version>1.7.25</slf4j.version>
Expand All @@ -486,7 +464,6 @@
<java-uuid-generator.version>3.1.5</java-uuid-generator.version>
<wiremock.version>2.14.0</wiremock.version>
<springfox.version>2.9.0</springfox.version>
<dcc-id.version>5.2.0</dcc-id.version>
<spring-security-oauth2.version>2.1.0.RELEASE</spring-security-oauth2.version>
<hibernate-native-json.version>0.4</hibernate-native-json.version>

Expand Down
17 changes: 5 additions & 12 deletions song-server/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@
<artifactId>lombok</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.icgc.dcc</groupId>
<artifactId>dcc-common-core</artifactId>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
Expand Down Expand Up @@ -194,13 +190,6 @@
<groupId>io.springfox</groupId>
<artifactId>springfox-swagger2</artifactId>
</dependency>

<!-- DCC - ID -->
<dependency>
<groupId>org.icgc.dcc</groupId>
<artifactId>dcc-id-client</artifactId>
</dependency>

<dependency>
<groupId>org.flywaydb</groupId>
<artifactId>flyway-core</artifactId>
Expand Down Expand Up @@ -240,7 +229,11 @@
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
</dependency>

<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>2.3.1</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

move this version definition into the <dependencyManagement> block of the root pom.xml. If this dependency is only used in song-server, then rip out the dependency from the dependencyManagement section and put it here. Also, use the version variable like ${jaxb.version}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

String constants are a valid part of the Java language. Don't be so afraid of them!

If we expect the definition of a constant to ever change, then YES, by all means, we should use a name rather than an inline value.

OBJECT_NAME_DELIMITER might change. NEWLINE might change (based upon different operating systems idea of what a new line should be).

But the definition of a something like COMMA should never change, and if it does, it's confusing.

All we accomplish by separating the definition of the constant from the code in this case is forcing the developer to do extra work to double check that COMMA is defined as ",".

Copy link
Contributor

@rtisma rtisma Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

i find that string constants are harder to read and make it hard to search for via the IDE. by using COMMA, you can inspect its usage easily. Regarding double checking, for something that sensitive, there should be tests in place that ensure it functions as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm confused. How is searching for "," any harder than searching for COMMA? If anything, it should be easier, since it's two less characters to type. :-)

There should always be tests. That doesn't mean that the code does what you think it does when you read it.:-)

But for consistency, I'll make you happy. :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

thank you!

</dependency>
</dependencies>

<build>
Expand Down
3 changes: 1 addition & 2 deletions song-server/src/main/java/bio/overture/song/server/controller/AnalysisController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package bio.overture.song.server.controller;

import static bio.overture.song.server.repository.search.IdSearchRequest.createIdSearchRequest;
import static org.icgc.dcc.common.core.util.Splitters.COMMA;
import static org.springframework.http.HttpHeaders.AUTHORIZATION;
import static org.springframework.http.MediaType.APPLICATION_JSON_UTF8_VALUE;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
Expand Down Expand Up @@ -82,7 +81,7 @@ public List<Analysis> getAnalysis(
@ApiParam(value = "Non-empty comma separated list of analysis states to filter by")
@RequestParam(value = "analysisStates", defaultValue = "PUBLISHED", required = false)
String analysisStates) {
return analysisService.getAnalysis(studyId, ImmutableSet.copyOf(COMMA.split(analysisStates)));
return analysisService.getAnalysis(studyId, ImmutableSet.copyOf(",".split(analysisStates)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a Splitters class with a no args constructor, that contains:
public static final Splitter COMMA = Splitter.on(",");
and likewise for any other splits.

}

/** [DCC-5726] - non-dynamic updates disabled until hibernate is properly integrated */
Expand Down
5 changes: 2 additions & 3 deletions ...n/java/bio/overture/song/server/controller/analysisType/AnalysisTypePageableResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,11 @@
import static bio.overture.song.server.model.enums.ModelAttributeNames.ID;
import static bio.overture.song.server.model.enums.ModelAttributeNames.NAME;
import static bio.overture.song.server.model.enums.ModelAttributeNames.VERSION;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.lang.Integer.parseInt;
import static java.util.Arrays.stream;
import static java.util.Objects.isNull;
import static lombok.AccessLevel.PRIVATE;
import static org.icgc.dcc.common.core.util.Joiners.COMMA;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;
import static org.springframework.data.domain.Sort.Direction.DESC;
import static org.springframework.util.StringUtils.isEmpty;

Expand Down Expand Up @@ -59,7 +58,7 @@ public class AnalysisTypePageableResolver implements HandlerMethodArgumentResolv
public static final String DEFAULT_SORT_VARIABLE = VERSION;
public static final Direction DEFAULT_SORT_ORDER = DESC;

private static final String ALLOWED_SORT_VARIABLES = COMMA.join(VERSION, NAME);
private static final String ALLOWED_SORT_VARIABLES = VERSION + "," + NAME;
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a no args constuctor Joiners class that contains the following:
public static final Joiner COMMA = Joiner.on(",");
likewise for any other characters to join on

private static final String ALLOWED_DIRECTION_VARIABLES =
stream(Direction.values())
.map(Enum::name)
Expand Down
5 changes: 2 additions & 3 deletions song-server/src/main/java/bio/overture/song/server/exceptions/ServerExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@
import static bio.overture.song.core.exceptions.ServerErrors.UNKNOWN_ERROR;
import static com.google.common.base.Throwables.getRootCause;
import static com.google.common.base.Throwables.getStackTraceAsString;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.lang.String.format;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.joining;
import static org.icgc.dcc.common.core.util.Splitters.NEWLINE;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;

import bio.overture.song.core.exceptions.ServerErrors;
import bio.overture.song.core.exceptions.ServerException;
Expand Down Expand Up @@ -172,7 +171,7 @@ private static String errorResponseBody(
}

private static List<String> getFullStackTraceList(Throwable t) {
return NEWLINE.splitToList(getStackTraceAsString(t)).stream()
return stream(getStackTraceAsString(t).split("\n"))
Copy link
Contributor

Choose a reason for hiding this comment

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

newline splitter

.map(String::trim)
.collect(toImmutableList());
}
Expand Down
2 changes: 1 addition & 1 deletion song-server/src/main/java/bio/overture/song/server/model/enums/InfoTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package bio.overture.song.server.model.enums;

import static java.lang.String.format;
import static org.icgc.dcc.common.core.util.stream.Streams.stream;
import static java.util.Arrays.stream;

import lombok.NonNull;
import lombok.RequiredArgsConstructor;
Expand Down
2 changes: 1 addition & 1 deletion song-server/src/main/java/bio/overture/song/server/model/enums/UploadStates.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

package bio.overture.song.server.model.enums;

import static bio.overture.song.core.utils.Streams.stream;
import static java.lang.String.format;
import static org.icgc.dcc.common.core.util.stream.Streams.stream;

import lombok.Getter;
import lombok.NonNull;
Expand Down
10 changes: 5 additions & 5 deletions song-server/src/main/java/bio/overture/song/server/service/AnalysisService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@
import static bio.overture.song.server.utils.JsonSchemas.REQUIRED;
import static bio.overture.song.server.utils.JsonSchemas.buildSchema;
import static bio.overture.song.server.utils.JsonSchemas.validateWithSchema;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.jayway.jsonpath.internal.Utils.join;
import static java.lang.String.format;
import static java.util.Objects.isNull;
import static org.icgc.dcc.common.core.util.Joiners.COMMA;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableMap;

import bio.overture.song.core.model.AnalysisTypeId;
import bio.overture.song.core.model.enums.AnalysisStates;
Expand Down Expand Up @@ -415,7 +415,7 @@ private void checkMissingFiles(String analysisId, List<FileEntity> files) {
"The following storage objectIds must be uploaded to the storage server before the "
+ "analysisId %s can be published: %s",
analysisId,
COMMA.join(missingFileIds));
join(",", missingFileIds));
Copy link
Contributor

Choose a reason for hiding this comment

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

use Joiners.COMMA

}

private List<String> saveCompositeEntities(
Expand Down Expand Up @@ -470,7 +470,7 @@ private void validateUpdateRequest(JsonNode request, AnalysisSchema analysisSche
try {
validateWithSchema(schema, request);
} catch (ValidationException e) {
throw buildServerException(getClass(), SCHEMA_VIOLATION, COMMA.join(e.getAllMessages()));
throw buildServerException(getClass(), SCHEMA_VIOLATION, join(",", e.getAllMessages()));
}
}

Expand Down
6 changes: 3 additions & 3 deletions song-server/src/main/java/bio/overture/song/server/service/AnalysisTypeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@
import static bio.overture.song.server.utils.JsonSchemas.buildSchema;
import static bio.overture.song.server.utils.JsonSchemas.validateWithSchema;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.isNull;
import static java.util.regex.Pattern.compile;
import static org.apache.commons.lang.StringUtils.isBlank;
import static org.icgc.dcc.common.core.util.Joiners.COMMA;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;

import bio.overture.song.core.model.AnalysisType;
import bio.overture.song.core.model.AnalysisTypeId;
Expand Down Expand Up @@ -233,7 +232,8 @@ private void validateAnalysisTypeSchema(JsonNode analysisTypeSchema) {
validateWithSchema(metaSchema, analysisTypeSchema);
buildSchema(analysisTypeSchema);
} catch (ValidationException e) {
throw buildServerException(getClass(), SCHEMA_VIOLATION, COMMA.join(e.getAllMessages()));
throw buildServerException(
getClass(), SCHEMA_VIOLATION, String.join(",", e.getAllMessages()));
} catch (SchemaException e) {
throw buildServerException(getClass(), MALFORMED_JSON_SCHEMA, e.getMessage());
}
Expand Down
2 changes: 1 addition & 1 deletion song-server/src/main/java/bio/overture/song/server/service/DonorService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import static bio.overture.song.core.exceptions.ServerException.checkServerOptional;
import static bio.overture.song.core.utils.Responses.OK;
import static com.google.common.base.Strings.isNullOrEmpty;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;
import static com.google.common.collect.ImmutableList.toImmutableList;

import bio.overture.song.server.model.entity.Donor;
import bio.overture.song.server.model.entity.composites.DonorWithSpecimens;
Expand Down
4 changes: 2 additions & 2 deletions song-server/src/main/java/bio/overture/song/server/service/ExportService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
import static bio.overture.song.core.model.ExportedPayload.createExportedPayload;
import static bio.overture.song.core.utils.JsonUtils.mapper;
import static bio.overture.song.server.service.AnalysisTypeService.resolveAnalysisTypeId;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.groupingBy;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableSet;

import bio.overture.song.core.model.ExportedPayload;
import bio.overture.song.core.model.enums.AnalysisStates;
Expand Down
6 changes: 3 additions & 3 deletions song-server/src/main/java/bio/overture/song/server/service/LegacyEntityService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

import static bio.overture.song.core.exceptions.ServerErrors.LEGACY_ENTITY_NOT_FOUND;
import static bio.overture.song.core.exceptions.ServerException.checkServer;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Sets.newHashSet;
import static java.lang.String.format;
import static org.icgc.dcc.common.core.util.Joiners.COMMA;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableSet;
import static java.lang.String.join;

import bio.overture.song.core.utils.JsonUtils;
import bio.overture.song.server.converter.LegacyEntityConverter;
Expand Down Expand Up @@ -103,7 +103,7 @@ private Set<String> extractQueryParameters(MultiValueMap<String, String> multiVa
private String buildLegacyEntityPageFilter(Set<String> filteredFieldNames) {
String filter = SQUIGGLY_ALL_FILTER;
if (!filteredFieldNames.isEmpty()) {
filter = format("%s,content[%s]", SQUIGGLY_ALL_FILTER, COMMA.join(filteredFieldNames));
filter = format("%s,content[%s]", SQUIGGLY_ALL_FILTER, join(",", filteredFieldNames));
}
return filter;
}
Expand Down
4 changes: 2 additions & 2 deletions song-server/src/main/java/bio/overture/song/server/service/StorageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import static bio.overture.song.core.exceptions.ServerException.checkServer;
import static bio.overture.song.core.utils.JsonUtils.readTree;
import static java.lang.Boolean.parseBoolean;
import static org.icgc.dcc.common.core.util.Joiners.SLASH;
import static java.lang.String.join;
import static org.springframework.http.HttpHeaders.AUTHORIZATION;
import static org.springframework.http.HttpMethod.GET;

Expand Down Expand Up @@ -136,6 +136,6 @@ private String getDownloadObjectUrl(String objectId) {
}

private static String joinUrl(String... path) {
return SLASH.join(path);
return join("/", path);
}
}
2 changes: 1 addition & 1 deletion song-server/src/main/java/bio/overture/song/server/service/StudyService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import static bio.overture.song.core.exceptions.ServerErrors.STUDY_ALREADY_EXISTS;
import static bio.overture.song.core.exceptions.ServerErrors.STUDY_ID_DOES_NOT_EXIST;
import static bio.overture.song.core.exceptions.ServerException.checkServer;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.lang.String.format;
import static java.lang.Thread.currentThread;
import static org.icgc.dcc.common.core.util.stream.Collectors.toImmutableList;

import bio.overture.song.server.model.entity.Study;
import bio.overture.song.server.repository.StudyRepository;
Expand Down
4 changes: 2 additions & 2 deletions song-server/src/main/java/bio/overture/song/server/service/ValidationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import static bio.overture.song.server.utils.JsonSchemas.buildSchema;
import static bio.overture.song.server.utils.JsonSchemas.validateWithSchema;
import static java.lang.String.format;
import static java.lang.String.join;
import static java.util.Objects.isNull;
import static org.apache.commons.lang.StringUtils.isBlank;
import static org.icgc.dcc.common.core.util.Joiners.COMMA;

import bio.overture.song.core.model.AnalysisTypeId;
import bio.overture.song.core.model.FileData;
Expand Down Expand Up @@ -99,7 +99,7 @@ public Optional<String> validate(@NonNull JsonNode payload) {
validateWithSchema(schema, payload);
}
} catch (ValidationException e) {
errors = COMMA.join(e.getAllMessages());
errors = join(",", e.getAllMessages());
log.error(errors);
}
return Optional.ofNullable(errors);
Expand Down
Loading