Skip to content

Commit

Permalink
chore(controllers): Return 404 and json content when objects are not …
Browse files Browse the repository at this point in the history
…found. (#123)
  • Loading branch information
Matt Duftler authored and Michael Graff committed Nov 15, 2017
1 parent 1edc513 commit 457724b
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

package com.netflix.kayenta.storage;

import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;

import java.util.List;
import java.util.Map;

public interface StorageService {
boolean servicesAccount(String accountName);
<T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException;
<T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException, NotFoundException;
<T> void storeObject(String accountName, ObjectType objectType, String objectKey, T obj, String filename, boolean isAnUpdate);
void deleteObject(String accountName, ObjectType objectType, String objectKey);
List<Map<String, Object>> listObjectKeys(String accountName, ObjectType objectType, List<String> applications, boolean skipIndex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.netflix.kayenta.security.AccountCredentialsRepository;
import com.netflix.kayenta.storage.ObjectType;
import com.netflix.kayenta.storage.StorageService;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import lombok.Builder;
import lombok.Getter;
import lombok.Singular;
Expand Down Expand Up @@ -114,13 +115,19 @@ public void ensureBucketExists(String accountName) {
}

@Override
public <T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException {
public <T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException, NotFoundException {
GoogleNamedAccountCredentials credentials = (GoogleNamedAccountCredentials)accountCredentialsRepository
.getOne(accountName)
.orElseThrow(() -> new IllegalArgumentException("Unable to resolve account " + accountName + "."));
Storage storage = credentials.getStorage();
String bucketName = credentials.getBucket();
StorageObject item = resolveSingularItem(objectType, objectKey, credentials, storage, bucketName);
StorageObject item;

try {
item = resolveSingularItem(objectType, objectKey, credentials, storage, bucketName);
} catch (IllegalArgumentException e) {
throw new NotFoundException(e.getMessage());
}

try {
StorageObject storageObject = storage.objects().get(bucketName, item.getName()).execute();
Expand All @@ -131,7 +138,7 @@ public <T> T loadObject(String accountName, ObjectType objectType, String object
HttpResponseException hre = (HttpResponseException)e;
log.error("Failed to load {} {}: {} {}", objectType.getGroup(), objectKey, hre.getStatusCode(), hre.getStatusMessage());
if (hre.getStatusCode() == 404) {
throw new IllegalArgumentException("No file at path " + item.getName() + ".");
throw new NotFoundException("No file at path " + item.getName() + ".");
}
}
throw new IllegalStateException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.netflix.kayenta.security.AccountCredentialsRepository;
import com.netflix.kayenta.storage.ObjectType;
import com.netflix.kayenta.storage.StorageService;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import com.squareup.okhttp.MediaType;
import com.squareup.okhttp.RequestBody;
import lombok.Builder;
Expand Down Expand Up @@ -69,7 +70,7 @@ public boolean servicesAccount(String accountName) {
}

@Override
public <T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException {
public <T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException, NotFoundException {
ConfigBinNamedAccountCredentials credentials = (ConfigBinNamedAccountCredentials)accountCredentialsRepository
.getOne(accountName)
.orElseThrow(() -> new IllegalArgumentException("Unable to resolve account " + accountName + "."));
Expand All @@ -81,7 +82,7 @@ public <T> T loadObject(String accountName, ObjectType objectType, String object
try {
json = remoteService.get(ownerApp, configType, objectKey);
} catch (RetrofitError e) {
throw new IllegalArgumentException("No such object named " + objectKey);
throw new NotFoundException("No such object named " + objectKey);
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.netflix.kayenta.security.AccountCredentialsRepository;
import com.netflix.kayenta.storage.ObjectType;
import com.netflix.kayenta.storage.StorageService;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import lombok.Builder;
import lombok.Getter;
import lombok.Singular;
Expand Down Expand Up @@ -65,7 +66,7 @@ public <T> T loadObject(String accountName, ObjectType objectType, String object
Object entry = credentials.getObjects().get(objectType).get(objectKey);

if (entry == null) {
throw new IllegalArgumentException("No such object named " + objectKey);
throw new NotFoundException("No such object named " + objectKey);
}

return (T)entry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.netflix.kayenta.security.AccountCredentialsRepository;
import com.netflix.kayenta.storage.ObjectType;
import com.netflix.kayenta.storage.StorageService;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import lombok.Builder;
import lombok.Getter;
import lombok.Singular;
Expand Down Expand Up @@ -101,13 +102,19 @@ public void ensureBucketExists(String accountName) {
}

@Override
public <T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException {
public <T> T loadObject(String accountName, ObjectType objectType, String objectKey) throws IllegalArgumentException, NotFoundException {
AwsNamedAccountCredentials credentials = (AwsNamedAccountCredentials)accountCredentialsRepository
.getOne(accountName)
.orElseThrow(() -> new IllegalArgumentException("Unable to resolve account " + accountName + "."));
AmazonS3 amazonS3 = credentials.getAmazonS3();
String bucket = credentials.getBucket();
String path = resolveSingularPath(objectType, objectKey, credentials, amazonS3, bucket);
String path;

try {
path = resolveSingularPath(objectType, objectKey, credentials, amazonS3, bucket);
} catch (IllegalArgumentException e) {
throw new NotFoundException(e.getMessage());
}

try {
S3Object s3Object = amazonS3.getObject(bucket, path);
Expand All @@ -116,7 +123,7 @@ public <T> T loadObject(String accountName, ObjectType objectType, String object
} catch (AmazonS3Exception e) {
log.error("Failed to load {} {}: {}", objectType.getGroup(), objectKey, e.getStatusCode());
if (e.getStatusCode() == 404) {
throw new IllegalArgumentException("No file at path " + path + ".");
throw new NotFoundException("No file at path " + path + ".");
}
throw e;
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@

package com.netflix.kayenta.controllers;

import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestControllerAdvice;

import java.util.Collections;
import java.util.Map;

@RestControllerAdvice
public class ControllerExceptionHandler {

Expand All @@ -29,4 +33,10 @@ public class ControllerExceptionHandler {
public String handleException(IllegalArgumentException e) {
return e.getMessage();
}

@ResponseStatus(HttpStatus.NOT_FOUND)
@ExceptionHandler(NotFoundException.class)
public Map handleException(NotFoundException e) {
return Collections.singletonMap("message", e.getMessage());
}
}

0 comments on commit 457724b

Please sign in to comment.