Skip to content
This repository was archived by the owner on Aug 4, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ sourceCompatibility = JavaVersion.VERSION_11

allprojects {
group = "bio.terra"
version = "0.9.0-SNAPSHOT"
version = "0.10.0-SNAPSHOT"
ext {
artifactGroup = "${group}.workspace"
swaggerOutputDir = "${buildDir}/swagger-code"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import bio.terra.workspace.generated.controller.WorkspaceApi;
import bio.terra.workspace.generated.model.*;
import bio.terra.workspace.service.datareference.DataReferenceService;
import bio.terra.workspace.service.datareference.exception.InvalidDataReferenceException;
import bio.terra.workspace.service.datareference.model.CloningInstructions;
import bio.terra.workspace.service.datareference.model.DataReference;
import bio.terra.workspace.service.datareference.model.DataReferenceRequest;
Expand Down Expand Up @@ -157,6 +158,7 @@ public ResponseEntity<DataReferenceDescription> createDataReference(
DataReferenceRequest.builder()
.workspaceId(id)
.name(body.getName())
.referenceDescription(body.getReferenceDescription())
.referenceType(referenceType)
.cloningInstructions(CloningInstructions.fromApiModel(body.getCloningInstructions()))
.referenceObject(snapshot)
Expand Down Expand Up @@ -209,6 +211,36 @@ public ResponseEntity<DataReferenceDescription> getDataReferenceByName(
return new ResponseEntity<>(ref.toApiModel(), HttpStatus.OK);
}

@Override
public ResponseEntity<Void> updateDataReference(
@PathVariable("id") UUID id,
@PathVariable("referenceId") UUID referenceId,
@RequestBody UpdateDataReferenceRequestBody body) {
AuthenticatedUserRequest userReq = getAuthenticatedInfo();

if (body.getName() == null && body.getReferenceDescription() == null) {
throw new InvalidDataReferenceException(
"Must specify name or referenceDescription to update.");
}

if (body.getName() != null) {
DataReferenceValidationUtils.validateReferenceName(body.getName());
}

logger.info(
String.format(
"Updating data reference by id %s in workspace %s for %s with body %s",
referenceId.toString(), id.toString(), userReq.getEmail(), body.toString()));
Comment thread
ddietterich marked this conversation as resolved.

dataReferenceService.updateDataReference(id, referenceId, body, userReq);
logger.info(
String.format(
"Updating data reference by id %s in workspace %s for %s with body %s",
referenceId.toString(), id.toString(), userReq.getEmail(), body.toString()));

return new ResponseEntity<>(HttpStatus.NO_CONTENT);
}

@Override
public ResponseEntity<Void> deleteDataReference(
@PathVariable("id") UUID workspaceId, @PathVariable("referenceId") UUID referenceId) {
Expand Down
57 changes: 52 additions & 5 deletions src/main/java/bio/terra/workspace/db/DataReferenceDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import bio.terra.workspace.common.exception.DataReferenceNotFoundException;
import bio.terra.workspace.common.exception.DuplicateDataReferenceException;
import bio.terra.workspace.db.exception.InvalidDaoRequestException;
import bio.terra.workspace.service.datareference.model.CloningInstructions;
import bio.terra.workspace.service.datareference.model.DataReference;
import bio.terra.workspace.service.datareference.model.DataReferenceRequest;
Expand All @@ -10,6 +11,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.List;
import java.util.Optional;
import java.util.StringJoiner;
import java.util.UUID;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -43,14 +45,15 @@ public DataReferenceDao(NamedParameterJdbcTemplate jdbcTemplate) {
public String createDataReference(DataReferenceRequest request, UUID referenceId)
throws DuplicateDataReferenceException {
String sql =
"INSERT INTO workspace_data_reference (workspace_id, reference_id, name, cloning_instructions, reference_type, reference) VALUES "
+ "(:workspace_id, :reference_id, :name, :cloning_instructions, :reference_type, cast(:reference AS json))";
"INSERT INTO workspace_data_reference (workspace_id, reference_id, name, reference_description, cloning_instructions, reference_type, reference) VALUES "
+ "(:workspace_id, :reference_id, :name, :reference_description, :cloning_instructions, :reference_type, cast(:reference AS json))";

MapSqlParameterSource params =
new MapSqlParameterSource()
.addValue("workspace_id", request.workspaceId().toString())
.addValue("reference_id", referenceId.toString())
.addValue("name", request.name())
.addValue("reference_description", request.referenceDescription())
.addValue("cloning_instructions", request.cloningInstructions().toSql())
.addValue("reference_type", request.referenceType().toSql())
.addValue("reference", request.referenceObject().toJson());
Expand All @@ -71,7 +74,7 @@ public String createDataReference(DataReferenceRequest request, UUID referenceId
/** Retrieve a data reference by ID from the DB. */
public DataReference getDataReference(UUID workspaceId, UUID referenceId) {
String sql =
"SELECT workspace_id, reference_id, name, cloning_instructions, reference_type, reference from workspace_data_reference where workspace_id = :workspace_id AND reference_id = :reference_id";
"SELECT workspace_id, reference_id, name, reference_description, cloning_instructions, reference_type, reference from workspace_data_reference where workspace_id = :workspace_id AND reference_id = :reference_id";

MapSqlParameterSource params =
new MapSqlParameterSource()
Expand All @@ -97,7 +100,7 @@ public DataReference getDataReference(UUID workspaceId, UUID referenceId) {
public DataReference getDataReferenceByName(
UUID workspaceId, DataReferenceType type, String name) {
String sql =
"SELECT workspace_id, reference_id, name, cloning_instructions, reference_type, reference from workspace_data_reference where workspace_id = :id AND reference_type = :type AND name = :name";
"SELECT workspace_id, reference_id, name, reference_description, cloning_instructions, reference_type, reference from workspace_data_reference where workspace_id = :id AND reference_type = :type AND name = :name";

MapSqlParameterSource params =
new MapSqlParameterSource()
Expand Down Expand Up @@ -135,6 +138,49 @@ public boolean isControlled(UUID workspaceId, UUID referenceId) {
}
}

public boolean updateDataReference(
UUID workspaceId, UUID referenceId, String name, String referenceDescription) {
if (name == null && referenceDescription == null) {
throw new InvalidDaoRequestException("Must specify name or referenceDescription to update.");
}

MapSqlParameterSource params =
new MapSqlParameterSource()
.addValue("id", referenceId.toString())
.addValue("workspace_id", workspaceId.toString())
.addValue("name", name)
.addValue("reference_description", referenceDescription);

StringJoiner updateStatement =
new StringJoiner(
", ",
"UPDATE workspace_data_reference SET ",
" WHERE reference_id = :id AND workspace_id = :workspace_id");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

using a StringJoiner here gets the job done and is a clever choice ... but it feels less readable imho than a StringBuilder. I would prefer readability ... but I also won't die on this hill.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I liked the StringJoiner because it's extensible for the future, if we ever have other updatable properties.

Copy link
Copy Markdown
Contributor Author

@zarsky-broad zarsky-broad Feb 9, 2021

Choose a reason for hiding this comment

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

(plus, using StringBuilder would mean doing a whole nested case thing to add the comma...)


if (name != null) {
updateStatement.add("name = :name");
}
if (referenceDescription != null) {
updateStatement.add("reference_description = :reference_description");
}
Comment thread
ddietterich marked this conversation as resolved.

int rowsAffected = jdbcTemplate.update(updateStatement.toString(), params);
boolean updated = rowsAffected > 0;

if (updated)
Comment thread
ddietterich marked this conversation as resolved.
logger.info(
String.format(
"Updated record for data reference %s in workspace %s",
referenceId.toString(), workspaceId.toString()));
else
logger.info(
String.format(
"Failed to update record for data reference %s in workspace %s",
referenceId.toString(), workspaceId.toString()));

return updated;
}

public boolean deleteDataReference(UUID workspaceId, UUID referenceId) {
MapSqlParameterSource params =
new MapSqlParameterSource()
Expand Down Expand Up @@ -164,7 +210,7 @@ public boolean deleteDataReference(UUID workspaceId, UUID referenceId) {
// should consider joining and listing those entries here.
public List<DataReference> enumerateDataReferences(UUID workspaceId, int offset, int limit) {
String sql =
"SELECT workspace_id, reference_id, name, cloning_instructions, reference_type, reference"
"SELECT workspace_id, reference_id, name, reference_description, cloning_instructions, reference_type, reference"
+ " FROM workspace_data_reference"
+ " WHERE workspace_id = :id"
+ " ORDER BY reference_id"
Expand All @@ -189,6 +235,7 @@ public List<DataReference> enumerateDataReferences(UUID workspaceId, int offset,
.workspaceId(UUID.fromString(rs.getString("workspace_id")))
.referenceId(UUID.fromString(rs.getString("reference_id")))
.name(rs.getString("name"))
.referenceDescription(rs.getString("reference_description"))
.referenceType(referenceType)
.cloningInstructions(CloningInstructions.fromSql(rs.getString("cloning_instructions")))
.referenceObject(deserializedReferenceObject)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package bio.terra.workspace.db.exception;

import bio.terra.common.exception.InternalServerErrorException;

public class InvalidDaoRequestException extends InternalServerErrorException {

public InvalidDaoRequestException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import bio.terra.workspace.common.exception.*;
import bio.terra.workspace.db.DataReferenceDao;
import bio.terra.workspace.generated.model.UpdateDataReferenceRequestBody;
import bio.terra.workspace.service.datareference.exception.ControlledResourceNotImplementedException;
import bio.terra.workspace.service.datareference.flight.CreateDataReferenceFlight;
import bio.terra.workspace.service.datareference.flight.DataReferenceFlightMapKeys;
Expand Down Expand Up @@ -98,6 +99,9 @@ public DataReference createDataReference(
userReq)
.addParameter(DataReferenceFlightMapKeys.WORKSPACE_ID, referenceRequest.workspaceId())
.addParameter(DataReferenceFlightMapKeys.NAME, referenceRequest.name())
.addParameter(
DataReferenceFlightMapKeys.REFERENCE_DESCRIPTION,
referenceRequest.referenceDescription())
.addParameter(
DataReferenceFlightMapKeys.REFERENCE_TYPE, referenceRequest.referenceType())
.addParameter(
Expand Down Expand Up @@ -128,6 +132,24 @@ public List<DataReference> enumerateDataReferences(
return dataReferenceDao.enumerateDataReferences(workspaceId, offset, limit);
}

@Traced
public void updateDataReference(
UUID workspaceId,
UUID referenceId,
UpdateDataReferenceRequestBody updateRequest,
AuthenticatedUserRequest userReq) {
workspaceService.validateWorkspaceAndAction(
userReq, workspaceId, SamConstants.SAM_WORKSPACE_WRITE_ACTION);

if (!dataReferenceDao.updateDataReference(
workspaceId,
referenceId,
updateRequest.getName(),
updateRequest.getReferenceDescription())) {
throw new DataReferenceNotFoundException("Data Reference not found.");
Comment thread
zarsky-broad marked this conversation as resolved.
}
}

/**
* Delete a data reference, or throw an exception if the specified reference does not exist.
* Verifies workspace existence and write permission before deleting the reference.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public StepResult doStep(FlightContext flightContext) throws RetryException {
UUID referenceId = workingMap.get(DataReferenceFlightMapKeys.REFERENCE_ID, UUID.class);
UUID workspaceId = inputMap.get(DataReferenceFlightMapKeys.WORKSPACE_ID, UUID.class);
String name = inputMap.get(DataReferenceFlightMapKeys.NAME, String.class);
String referenceDescription =
inputMap.get(DataReferenceFlightMapKeys.REFERENCE_DESCRIPTION, String.class);
DataReferenceType type =
inputMap.get(DataReferenceFlightMapKeys.REFERENCE_TYPE, DataReferenceType.class);
CloningInstructions cloningInstructions =
Expand All @@ -43,6 +45,7 @@ public StepResult doStep(FlightContext flightContext) throws RetryException {
DataReferenceRequest.builder()
.workspaceId(workspaceId)
.name(name)
.referenceDescription(referenceDescription)
.referenceType(type)
.cloningInstructions(cloningInstructions)
.referenceObject(referenceObject)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ public final class DataReferenceFlightMapKeys {
public static final String REFERENCE_ID = "referenceId";
public static final String WORKSPACE_ID = "workspaceId";
public static final String NAME = "name";
public static final String REFERENCE_DESCRIPTION = "referenceDescription";
public static final String REFERENCE_TYPE = "referenceType";
public static final String CLONING_INSTRUCTIONS = "cloningInstructions";
public static final String REFERENCE_OBJECT = "referenceObject";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import bio.terra.workspace.generated.model.DataReferenceDescription;
import com.google.auto.value.AutoValue;
import java.util.UUID;
import javax.annotation.Nullable;

/**
* Internal representation of an uncontrolled data reference.
Expand All @@ -21,6 +22,10 @@ public abstract class DataReference {
/** Name of the reference. Names are unique per workspace, per reference type. */
public abstract String name();

/** Description of the data reference. */
@Nullable
public abstract String referenceDescription();

/** Type of this data reference. */
public abstract DataReferenceType referenceType();

Expand All @@ -35,6 +40,7 @@ public DataReferenceDescription toApiModel() {
return new DataReferenceDescription()
.referenceId(referenceId())
.name(name())
.referenceDescription(referenceDescription())
.workspaceId(workspaceId())
.referenceType(referenceType().toApiModel())
.reference(((SnapshotReference) referenceObject()).toApiModel())
Expand All @@ -53,6 +59,8 @@ public abstract static class Builder {

public abstract DataReference.Builder name(String value);

public abstract DataReference.Builder referenceDescription(String value);

public abstract DataReference.Builder referenceType(DataReferenceType value);

public abstract DataReference.Builder cloningInstructions(CloningInstructions value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.google.auto.value.AutoValue;
import java.util.UUID;
import javax.annotation.Nullable;

/**
* This is an internal representation of a request to create a data reference.
Expand All @@ -22,6 +23,10 @@ public abstract class DataReferenceRequest {
*/
public abstract String name();

/** Description of the reference. */
@Nullable
public abstract String referenceDescription();

/** Type of this data reference. */
public abstract DataReferenceType referenceType();

Expand All @@ -41,6 +46,8 @@ public abstract static class Builder {

public abstract DataReferenceRequest.Builder name(String value);

public abstract DataReferenceRequest.Builder referenceDescription(String value);

public abstract DataReferenceRequest.Builder referenceType(DataReferenceType value);

public abstract DataReferenceRequest.Builder cloningInstructions(CloningInstructions value);
Expand Down
31 changes: 31 additions & 0 deletions src/main/resources/api/service_openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,25 @@ paths:
$ref: '#/components/responses/NotFound'
'500':
$ref: '#/components/responses/ServerError'
patch:
summary: Update name or description of a data reference in a workspace.
operationId: updateDataReference
tags: [Workspace]
requestBody:
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/UpdateDataReferenceRequestBody'
responses:
'204':
description: OK
'403':
$ref: '#/components/responses/PermissionDenied'
'404':
$ref: '#/components/responses/NotFound'
'500':
$ref: '#/components/responses/ServerError'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This satisfies the current Jira ticket, but from an API design perspective it's awkward that we can create and get entire data references, but we can only edit the name/description of those references - we can't edit the innards, such as pointing an existing reference at a different snapshot id. I'm not sure the best way to handle this - possibly just explaining it in the API description.

delete:
summary: Deletes a data reference from a workspace.
operationId: deleteDataReference
Expand Down Expand Up @@ -606,6 +625,8 @@ components:
properties:
name:
$ref: "#/components/schemas/Name"
referenceDescription:
type: string
resourceId:
description: The ID of the resource
type: string
Expand All @@ -617,6 +638,14 @@ components:
cloningInstructions:
$ref: '#/components/schemas/CloningInstructionsEnum'

UpdateDataReferenceRequestBody:
type: object
properties:
name:
$ref: "#/components/schemas/Name"
referenceDescription:
type: string

DataReferenceDescription:
type: object
required: [referenceId, name, workspaceId, cloningInstructions]
Expand All @@ -628,6 +657,8 @@ components:
name:
description: The name of the data reference; used to refer to the reference
type: string
referenceDescription:
type: string
workspaceId:
description: The ID of the workspace containing the reference
type: string
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/db/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
<include file="changesets/20201028_feature_toggle.yaml" relativeToChangelogFile="true"/>
<include file="changesets/20201104_reference_index.yaml" relativeToChangelogFile="true"/>
<include file="changesets/20201204_json_to_jsonb.yaml" relativeToChangelogFile="true"/>
<include file="changesets/20210205_add_description.yaml" relativeToChangelogFile="true"/>
</databaseChangeLog>
Loading