Skip to content

Commit

Permalink
feat(openapi): openapi v3 updates
Browse files Browse the repository at this point in the history
* Added flag to HEAD endpoint to consider (or not) soft deleted entities
* GET operations will no longer return default key aspects (instead 404)
  * reflects whether the entity actually exists in the database
* better error messages on invalid urns
* refactor generic controller from models to servlet
  • Loading branch information
david-leifker committed Jun 14, 2024
1 parent bb44c4c commit 2c31718
Show file tree
Hide file tree
Showing 13 changed files with 276 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public EntityResponse getV2(
@Nonnull
@Deprecated
public Entity get(@Nonnull OperationContext opContext, @Nonnull final Urn urn) {
return entityService.getEntity(opContext, urn, ImmutableSet.of());
return entityService.getEntity(opContext, urn, ImmutableSet.of(), true);
}

@Nonnull
Expand Down Expand Up @@ -175,7 +175,7 @@ public Map<Urn, EntityResponse> batchGetVersionedV2(
@Deprecated
public Map<Urn, Entity> batchGet(
@Nonnull OperationContext opContext, @Nonnull final Set<Urn> urns) {
return entityService.getEntities(opContext, urns, ImmutableSet.of());
return entityService.getEntities(opContext, urns, ImmutableSet.of(), true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ public RecordTemplate getLatestAspect(
public Map<Urn, List<RecordTemplate>> getLatestAspects(
@Nonnull OperationContext opContext,
@Nonnull final Set<Urn> urns,
@Nonnull final Set<String> aspectNames) {
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect) {

Map<EntityAspectIdentifier, EntityAspect> batchGetResults =
getLatestAspect(opContext, urns, aspectNames);
Expand All @@ -223,15 +224,17 @@ public Map<Urn, List<RecordTemplate>> getLatestAspects(
urnToAspects.putIfAbsent(urn, new ArrayList<>());
}

// Add "key" aspects for each urn. TODO: Replace this with a materialized key aspect.
urnToAspects
.keySet()
.forEach(
key -> {
final RecordTemplate keyAspect =
EntityApiUtils.buildKeyAspect(opContext.getEntityRegistry(), key);
urnToAspects.get(key).add(keyAspect);
});
if (alwaysIncludeKeyAspect) {
// Add "key" aspects for each urn. TODO: Replace this with a materialized key aspect.
urnToAspects
.keySet()
.forEach(
key -> {
final RecordTemplate keyAspect =
EntityApiUtils.buildKeyAspect(opContext.getEntityRegistry(), key);
urnToAspects.get(key).add(keyAspect);
});
}

List<SystemAspect> systemAspects =
EntityUtils.toSystemAspects(
Expand Down Expand Up @@ -328,9 +331,12 @@ public EntityResponse getEntityV2(
@Nonnull OperationContext opContext,
@Nonnull final String entityName,
@Nonnull final Urn urn,
@Nonnull final Set<String> aspectNames)
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect)
throws URISyntaxException {
return getEntitiesV2(opContext, entityName, Collections.singleton(urn), aspectNames).get(urn);
return getEntitiesV2(
opContext, entityName, Collections.singleton(urn), aspectNames, alwaysIncludeKeyAspect)
.get(urn);
}

/**
Expand All @@ -348,9 +354,12 @@ public Map<Urn, EntityResponse> getEntitiesV2(
@Nonnull OperationContext opContext,
@Nonnull final String entityName,
@Nonnull final Set<Urn> urns,
@Nonnull final Set<String> aspectNames)
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect)
throws URISyntaxException {
return getLatestEnvelopedAspects(opContext, urns, aspectNames).entrySet().stream()
return getLatestEnvelopedAspects(opContext, urns, aspectNames, alwaysIncludeKeyAspect)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
Expand All @@ -370,9 +379,13 @@ public Map<Urn, EntityResponse> getEntitiesV2(
public Map<Urn, EntityResponse> getEntitiesVersionedV2(
@Nonnull OperationContext opContext,
@Nonnull final Set<VersionedUrn> versionedUrns,
@Nonnull final Set<String> aspectNames)
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect)
throws URISyntaxException {
return getVersionedEnvelopedAspects(opContext, versionedUrns, aspectNames).entrySet().stream()
return getVersionedEnvelopedAspects(
opContext, versionedUrns, aspectNames, alwaysIncludeKeyAspect)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
Expand All @@ -388,7 +401,10 @@ public Map<Urn, EntityResponse> getEntitiesVersionedV2(
*/
@Override
public Map<Urn, List<EnvelopedAspect>> getLatestEnvelopedAspects(
@Nonnull OperationContext opContext, @Nonnull Set<Urn> urns, @Nonnull Set<String> aspectNames)
@Nonnull OperationContext opContext,
@Nonnull Set<Urn> urns,
@Nonnull Set<String> aspectNames,
boolean alwaysIncludeKeyAspect)
throws URISyntaxException {

final Set<EntityAspectIdentifier> dbKeys =
Expand All @@ -404,7 +420,7 @@ public Map<Urn, List<EnvelopedAspect>> getLatestEnvelopedAspects(
.flatMap(List::stream)
.collect(Collectors.toSet());

return getCorrespondingAspects(opContext, dbKeys, urns);
return getCorrespondingAspects(opContext, dbKeys, urns, alwaysIncludeKeyAspect);
}

/**
Expand All @@ -419,7 +435,8 @@ public Map<Urn, List<EnvelopedAspect>> getLatestEnvelopedAspects(
public Map<Urn, List<EnvelopedAspect>> getVersionedEnvelopedAspects(
@Nonnull OperationContext opContext,
@Nonnull Set<VersionedUrn> versionedUrns,
@Nonnull Set<String> aspectNames)
@Nonnull Set<String> aspectNames,
boolean alwaysIncludeKeyAspect)
throws URISyntaxException {

Map<String, Map<String, Long>> urnAspectVersionMap =
Expand Down Expand Up @@ -466,11 +483,15 @@ public Map<Urn, List<EnvelopedAspect>> getVersionedEnvelopedAspects(
versionedUrns.stream()
.map(versionedUrn -> versionedUrn.getUrn().toString())
.map(UrnUtils::getUrn)
.collect(Collectors.toSet()));
.collect(Collectors.toSet()),
alwaysIncludeKeyAspect);
}

private Map<Urn, List<EnvelopedAspect>> getCorrespondingAspects(
@Nonnull OperationContext opContext, Set<EntityAspectIdentifier> dbKeys, Set<Urn> urns) {
@Nonnull OperationContext opContext,
Set<EntityAspectIdentifier> dbKeys,
Set<Urn> urns,
boolean alwaysIncludeKeyAspect) {

final Map<EntityAspectIdentifier, EnvelopedAspect> envelopedAspectMap =
getEnvelopedAspects(opContext, dbKeys);
Expand All @@ -487,11 +508,14 @@ private Map<Urn, List<EnvelopedAspect>> getCorrespondingAspects(
for (Urn urn : urns) {
List<EnvelopedAspect> aspects =
urnToAspects.getOrDefault(urn.toString(), Collections.emptyList());

EnvelopedAspect keyAspect =
EntityUtils.getKeyEnvelopedAspect(urn, opContext.getEntityRegistry());
// Add key aspect if it does not exist in the returned aspects
if (aspects.isEmpty()
|| aspects.stream().noneMatch(aspect -> keyAspect.getName().equals(aspect.getName()))) {
if (alwaysIncludeKeyAspect
&& (aspects.isEmpty()
|| aspects.stream()
.noneMatch(aspect -> keyAspect.getName().equals(aspect.getName())))) {
result.put(
urn, ImmutableList.<EnvelopedAspect>builder().addAll(aspects).add(keyAspect).build());
} else {
Expand Down Expand Up @@ -1532,8 +1556,11 @@ public ListUrnsResult listUrns(
public Entity getEntity(
@Nonnull OperationContext opContext,
@Nonnull final Urn urn,
@Nonnull final Set<String> aspectNames) {
return getEntities(opContext, Collections.singleton(urn), aspectNames).values().stream()
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect) {
return getEntities(opContext, Collections.singleton(urn), aspectNames, alwaysIncludeKeyAspect)
.values()
.stream()
.findFirst()
.orElse(null);
}
Expand All @@ -1552,12 +1579,15 @@ public Entity getEntity(
public Map<Urn, Entity> getEntities(
@Nonnull OperationContext opContext,
@Nonnull final Set<Urn> urns,
@Nonnull Set<String> aspectNames) {
@Nonnull Set<String> aspectNames,
boolean alwaysIncludeKeyAspect) {
log.debug("Invoked getEntities with urns {}, aspects {}", urns, aspectNames);
if (urns.isEmpty()) {
return Collections.emptyMap();
}
return getSnapshotUnions(opContext, urns, aspectNames).entrySet().stream()
return getSnapshotUnions(opContext, urns, aspectNames, alwaysIncludeKeyAspect)
.entrySet()
.stream()
.collect(
Collectors.toMap(Map.Entry::getKey, entry -> EntityUtils.toEntity(entry.getValue())));
}
Expand Down Expand Up @@ -1708,8 +1738,11 @@ public void ingestEntity(
protected Map<Urn, Snapshot> getSnapshotUnions(
@Nonnull OperationContext opContext,
@Nonnull final Set<Urn> urns,
@Nonnull final Set<String> aspectNames) {
return getSnapshotRecords(opContext, urns, aspectNames).entrySet().stream()
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect) {
return getSnapshotRecords(opContext, urns, aspectNames, alwaysIncludeKeyAspect)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey, entry -> EntityUtils.toSnapshotUnion(entry.getValue())));
Expand All @@ -1719,8 +1752,11 @@ protected Map<Urn, Snapshot> getSnapshotUnions(
protected Map<Urn, RecordTemplate> getSnapshotRecords(
@Nonnull OperationContext opContext,
@Nonnull final Set<Urn> urns,
@Nonnull final Set<String> aspectNames) {
return getLatestAspectUnions(opContext, urns, aspectNames).entrySet().stream()
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect) {
return getLatestAspectUnions(opContext, urns, aspectNames, alwaysIncludeKeyAspect)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
Expand All @@ -1731,8 +1767,11 @@ protected Map<Urn, RecordTemplate> getSnapshotRecords(
protected Map<Urn, List<UnionTemplate>> getLatestAspectUnions(
@Nonnull OperationContext opContext,
@Nonnull final Set<Urn> urns,
@Nonnull final Set<String> aspectNames) {
return this.getLatestAspects(opContext, urns, aspectNames).entrySet().stream()
@Nonnull final Set<String> aspectNames,
boolean alwaysIncludeKeyAspect) {
return this.getLatestAspects(opContext, urns, aspectNames, alwaysIncludeKeyAspect)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
Expand Down Expand Up @@ -2001,7 +2040,7 @@ public Set<Urn> exists(
} else {
// Additionally exclude status.removed == true
Map<Urn, List<RecordTemplate>> statusResult =
getLatestAspects(opContext, existing, Set.of(STATUS_ASPECT_NAME));
getLatestAspects(opContext, existing, Set.of(STATUS_ASPECT_NAME), false);
return existing.stream()
.filter(
urn ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void testIngestGetEntity() throws Exception {

// 2. Retrieve Entity
com.linkedin.entity.Entity readEntity =
_entityServiceImpl.getEntity(opContext, entityUrn, Collections.emptySet());
_entityServiceImpl.getEntity(opContext, entityUrn, Collections.emptySet(), true);

// 3. Compare Entity Objects
assertEquals(
Expand Down Expand Up @@ -206,7 +206,7 @@ public void testAddKey() throws Exception {

// 2. Retrieve Entity
com.linkedin.entity.Entity readEntity =
_entityServiceImpl.getEntity(opContext, entityUrn, Collections.emptySet());
_entityServiceImpl.getEntity(opContext, entityUrn, Collections.emptySet(), true);

// 3. Compare Entity Objects
assertEquals(
Expand Down Expand Up @@ -261,7 +261,7 @@ public void testIngestGetEntities() throws Exception {
// 2. Retrieve Entities
Map<Urn, Entity> readEntities =
_entityServiceImpl.getEntities(
opContext, ImmutableSet.of(entityUrn1, entityUrn2), Collections.emptySet());
opContext, ImmutableSet.of(entityUrn1, entityUrn2), Collections.emptySet(), true);

// 3. Compare Entity Objects

Expand Down
8 changes: 0 additions & 8 deletions metadata-service/openapi-servlet/models/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ dependencies {
implementation project(':entity-registry')
implementation project(':metadata-operation-context')
implementation project(':metadata-auth:auth-api')
implementation project(':metadata-service:auth-impl')
implementation project(':metadata-io')

implementation externalDependency.springWeb
implementation(externalDependency.springDocUI) {
exclude group: 'org.springframework.boot'
}
implementation externalDependency.swaggerAnnotations

implementation externalDependency.jacksonDataBind
implementation externalDependency.httpClient
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.datahubproject.openapi.exception;

import java.net.URISyntaxException;

public class InvalidUrnException extends URISyntaxException {
public InvalidUrnException(String input, String reason) {
super(input, reason);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.datahubproject.openapi;

import io.datahubproject.openapi.exception.InvalidUrnException;
import java.util.Map;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.ConversionNotSupportedException;
import org.springframework.core.Ordered;
Expand All @@ -23,4 +25,9 @@ public GlobalControllerExceptionHandler() {
public ResponseEntity<String> handleConflict(RuntimeException ex) {
return new ResponseEntity<>(ex.getMessage(), HttpStatus.BAD_REQUEST);
}

@ExceptionHandler(InvalidUrnException.class)
public static ResponseEntity<Map<String, String>> handleUrnException(InvalidUrnException e) {
return new ResponseEntity<>(Map.of("error", e.getMessage()), HttpStatus.BAD_REQUEST);
}
}
Loading

0 comments on commit 2c31718

Please sign in to comment.