Skip to content

Commit

Permalink
[WOR-1696] Soft delete PAOs (#101)
Browse files Browse the repository at this point in the history
  • Loading branch information
marctalbott authored Jul 8, 2024
1 parent 6f5ae5b commit b3425da
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 349 deletions.
11 changes: 11 additions & 0 deletions common/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ paths:
$ref: '#/components/responses/ServerError'
get:
summary: Retrieve a Policy Attribute Object
parameters:
- $ref: '#/components/parameters/IncludeDeleted'
operationId: getPao
tags: [Tps]
responses:
Expand Down Expand Up @@ -489,6 +491,15 @@ components:
schema:
$ref: '#/components/schemas/TpsDepth'

IncludeDeleted:
name: includeDeleted
in: query
description: |
Include deleted policy attribute objects in the response. Defaults to false.
required: false
schema:
type: boolean

responses:
# Error Responses
BadRequest:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,11 @@ private ApiTpsPolicyExplanation convertExplanation(ExplainGraphNode node) {
}

@Override
public ResponseEntity<ApiTpsPaoGetResult> getPao(UUID objectId) {
Pao pao = paoService.getPao(objectId);
public ResponseEntity<ApiTpsPaoGetResult> getPao(UUID objectId, Boolean includeDeleted) {
if (includeDeleted == null) {
includeDeleted = false;
}
Pao pao = paoService.getPao(objectId, includeDeleted);
ApiTpsPaoGetResult result = ConversionUtils.paoToApi(pao);
MetricsUtils.incrementPaoGet();
return new ResponseEntity<>(result, HttpStatus.OK);
Expand Down
20 changes: 9 additions & 11 deletions service/src/main/java/bio/terra/policy/db/PaoDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -103,11 +102,6 @@ public void createPao(
effectiveSetId);
}

@WriteTransaction
public void deletePaos(Collection<DbPao> paos) {
paos.forEach((DbPao pao) -> removeDbPao(pao));
}

/**
* Set the 'deleted' field on the PAO to true.
*
Expand All @@ -122,8 +116,8 @@ public void markPaoDeleted(UUID objectId) {
}

@ReadTransaction
public Pao getPao(UUID objectId) {
DbPao dbPao = getDbPao(objectId);
public Pao getPao(UUID objectId, boolean includeDeleted) {
DbPao dbPao = getDbPao(objectId, includeDeleted);
Map<String, PolicyInputs> attributeSetMap =
getAttributeSets(List.of(dbPao.attributeSetId(), dbPao.effectiveSetId()));
return Pao.fromDb(dbPao, attributeSetMap);
Expand Down Expand Up @@ -236,7 +230,7 @@ private void updatePao(GraphNode change) {
PolicyInputs effectiveAttributes = change.getEffectivePolicyAttributes();

// Get the dbPao and the attribute sets from the db for comparison
DbPao dbPao = getDbPao(pao.getObjectId());
DbPao dbPao = getDbPao(pao.getObjectId(), true);
Map<String, PolicyInputs> attributeSetMap =
getAttributeSets(List.of(dbPao.attributeSetId(), dbPao.effectiveSetId()));
PolicyInputs dbAttributes = attributeSetMap.get(dbPao.attributeSetId());
Expand Down Expand Up @@ -395,13 +389,17 @@ private void deleteAttributeSet(String setId) {
tpsJdbcTemplate.update(sql, params);
}

public DbPao getDbPao(UUID objectId) {
final String sql =
public DbPao getDbPao(UUID objectId, boolean includeDeleted) {
String sql =
"""
SELECT object_id, component, object_type, attribute_set_id, effective_set_id, sources, deleted, created, last_updated
FROM policy_object WHERE object_id = :object_id
""";

if (!includeDeleted) {
sql += " AND (deleted is null or not deleted)";
}

MapSqlParameterSource params =
new MapSqlParameterSource().addValue("object_id", objectId.toString());

Expand Down
24 changes: 12 additions & 12 deletions service/src/main/java/bio/terra/policy/service/pao/PaoService.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
import bio.terra.policy.common.exception.InvalidInputException;
import bio.terra.policy.common.model.PolicyInput;
import bio.terra.policy.common.model.PolicyInputs;
import bio.terra.policy.db.DbPao;
import bio.terra.policy.db.PaoDao;
import bio.terra.policy.service.pao.graph.DeleteWalker;
import bio.terra.policy.service.pao.graph.ExplainWalker;
import bio.terra.policy.service.pao.graph.Walker;
import bio.terra.policy.service.pao.graph.model.ExplainGraph;
Expand Down Expand Up @@ -70,9 +68,6 @@ public void createPao(
public void deletePao(UUID objectId) {
logger.info("Delete PAO id {}", objectId);
paoDao.markPaoDeleted(objectId);
DeleteWalker walker = new DeleteWalker(paoDao, objectId);
Set<DbPao> toRemove = walker.findRemovablePaos();
paoDao.deletePaos(toRemove);
}

/**
Expand All @@ -87,9 +82,14 @@ public ExplainGraph explainPao(UUID objectId, int depth) {
return walker.getExplainGraph();
}

public Pao getPao(UUID objectId) {
public Pao getPao(UUID objectId, boolean includeDeleted) {
logger.info("Get PAO id {}", objectId);
return paoDao.getPao(objectId);

return paoDao.getPao(objectId, includeDeleted);
}

public Pao getPao(UUID objectId) {
return getPao(objectId, false);
}

@ReadTransaction
Expand All @@ -114,7 +114,7 @@ public PolicyUpdateResult linkSourcePao(
logger.info(
"LinkSourcePao: dependent {} source {} mode {}", objectId, sourceObjectId, updateMode);

Pao targetPao = paoDao.getPao(objectId);
Pao targetPao = paoDao.getPao(objectId, false);
boolean newSource = targetPao.getSourceObjectIds().add(sourceObjectId);

// We didn't actually change the source list, so we are done
Expand Down Expand Up @@ -164,8 +164,8 @@ public PolicyUpdateResult mergeFromPao(
"Merge from PAO id {} to {} mode {}", sourceObjectId, destinationObjectId, updateMode);

// Step 0: get the paos. This will throw if they are not present
Pao sourcePao = paoDao.getPao(sourceObjectId);
Pao destinationPao = paoDao.getPao(destinationObjectId);
Pao sourcePao = paoDao.getPao(sourceObjectId, false);
Pao destinationPao = paoDao.getPao(destinationObjectId, false);

// If the source and destination are the same PAO, there is nothing to do
if (sourceObjectId.equals(destinationObjectId)) {
Expand Down Expand Up @@ -211,7 +211,7 @@ public PolicyUpdateResult replacePao(
replacementAttributes,
updateMode);
validatePolicyInputs(replacementAttributes);
Pao targetPao = paoDao.getPao(targetPaoId);
Pao targetPao = paoDao.getPao(targetPaoId, false);
return updateAttributesWorker(replacementAttributes, targetPao, updateMode);
}

Expand All @@ -236,7 +236,7 @@ public PolicyUpdateResult updatePao(
removeAttributes,
updateMode);

Pao targetPao = paoDao.getPao(targetPaoId);
Pao targetPao = paoDao.getPao(targetPaoId, false);
PolicyInputs attributesToUpdate = new PolicyInputs(targetPao.getAttributes());

// We do the removes first, so we don't remove newly added things
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public ExplainGraph getExplainGraph() {
private Pao getPao(UUID objectId) {
Pao pao = paoMap.get(objectId);
if (pao == null) {
pao = paoDao.getPao(objectId);
pao = paoDao.getPao(objectId, true);
paoMap.put(objectId, pao);
}
return pao;
Expand Down
Loading

0 comments on commit b3425da

Please sign in to comment.