From cda190f3fa8148ce43522d6e04951a3db4955ba9 Mon Sep 17 00:00:00 2001 From: Lionel Montrieux Date: Wed, 1 Feb 2017 11:42:27 +0100 Subject: [PATCH] ARUHA-523 Use DB's uniqueness constraint to prevent duplicate storages --- .../db/TimelineDbRepositoryTest.java | 16 +++++++++----- .../DuplicatedStorageIdException.java | 22 +++++++++++++++++++ .../repository/db/StorageDbRepository.java | 7 +++++- .../nakadi/service/StorageService.java | 11 +++------- .../nakadi/service/StorageServiceTest.java | 1 - 5 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 src/main/java/org/zalando/nakadi/exceptions/DuplicatedStorageIdException.java diff --git a/src/acceptance-test/java/org/zalando/nakadi/repository/db/TimelineDbRepositoryTest.java b/src/acceptance-test/java/org/zalando/nakadi/repository/db/TimelineDbRepositoryTest.java index 494b2c9d95..918376db40 100644 --- a/src/acceptance-test/java/org/zalando/nakadi/repository/db/TimelineDbRepositoryTest.java +++ b/src/acceptance-test/java/org/zalando/nakadi/repository/db/TimelineDbRepositoryTest.java @@ -14,6 +14,7 @@ import org.zalando.nakadi.domain.Storage; import org.zalando.nakadi.domain.Timeline; import org.zalando.nakadi.exceptions.DuplicatedEventTypeNameException; +import org.zalando.nakadi.exceptions.DuplicatedStorageIdException; import org.zalando.nakadi.exceptions.InternalNakadiException; import org.zalando.nakadi.utils.TestUtils; @@ -36,7 +37,8 @@ public void setUp() { } @Test - public void testTimelineCreated() throws InternalNakadiException, DuplicatedEventTypeNameException { + public void testTimelineCreated() + throws InternalNakadiException, DuplicatedEventTypeNameException, DuplicatedStorageIdException { final Storage storage = sRepository.createStorage( StorageDbRepositoryTest.createStorage("default", "test", "path")); final EventType testEt = eRepository.saveEventType(TestUtils.buildDefaultEventType()); @@ -54,7 +56,8 @@ public void testTimelineCreated() throws InternalNakadiException, DuplicatedEven } @Test - public void testTimelineUpdate() throws InternalNakadiException, DuplicatedEventTypeNameException { + public void testTimelineUpdate() + throws InternalNakadiException, DuplicatedEventTypeNameException, DuplicatedStorageIdException { final Storage storage = sRepository.createStorage( StorageDbRepositoryTest.createStorage("default", "test", "path")); final EventType testEt = eRepository.saveEventType(TestUtils.buildDefaultEventType()); @@ -79,7 +82,8 @@ public void testTimelineUpdate() throws InternalNakadiException, DuplicatedEvent } @Test(expected = DuplicateKeyException.class) - public void testDuplicateOrderNotAllowed() throws InternalNakadiException, DuplicatedEventTypeNameException { + public void testDuplicateOrderNotAllowed() + throws InternalNakadiException, DuplicatedEventTypeNameException, DuplicatedStorageIdException { final Storage storage = sRepository.createStorage( StorageDbRepositoryTest.createStorage("default", "test", "path")); final EventType testEt = eRepository.saveEventType(TestUtils.buildDefaultEventType()); @@ -93,7 +97,8 @@ public void testDuplicateOrderNotAllowed() throws InternalNakadiException, Dupli } @Test - public void testListTimelinesOrdered() throws InternalNakadiException, DuplicatedEventTypeNameException { + public void testListTimelinesOrdered() + throws InternalNakadiException, DuplicatedEventTypeNameException, DuplicatedStorageIdException { final Storage storage = sRepository.createStorage( StorageDbRepositoryTest.createStorage("default", "test", "path")); final EventType testEt = eRepository.saveEventType(TestUtils.buildDefaultEventType()); @@ -111,7 +116,8 @@ public void testListTimelinesOrdered() throws InternalNakadiException, Duplicate } @Test - public void testTimelineDeleted() throws InternalNakadiException, DuplicatedEventTypeNameException { + public void testTimelineDeleted() + throws InternalNakadiException, DuplicatedEventTypeNameException, DuplicatedStorageIdException { final Storage storage = sRepository.createStorage( StorageDbRepositoryTest.createStorage("default", "test", "path")); final EventType testEt = eRepository.saveEventType(TestUtils.buildDefaultEventType()); diff --git a/src/main/java/org/zalando/nakadi/exceptions/DuplicatedStorageIdException.java b/src/main/java/org/zalando/nakadi/exceptions/DuplicatedStorageIdException.java new file mode 100644 index 0000000000..cdae141c85 --- /dev/null +++ b/src/main/java/org/zalando/nakadi/exceptions/DuplicatedStorageIdException.java @@ -0,0 +1,22 @@ +package org.zalando.nakadi.exceptions; + +import javax.ws.rs.core.Response; + +public class DuplicatedStorageIdException extends NakadiException { + public DuplicatedStorageIdException(final String message) { + super(message); + } + + public DuplicatedStorageIdException(final String msg, final Exception cause) { + super(msg, cause); + } + + public DuplicatedStorageIdException(final String msg, final String problemMessage, final Exception cause) { + super(msg, problemMessage, cause); + } + + @Override + protected Response.StatusType getStatus() { + return Response.Status.CONFLICT; + } +} diff --git a/src/main/java/org/zalando/nakadi/repository/db/StorageDbRepository.java b/src/main/java/org/zalando/nakadi/repository/db/StorageDbRepository.java index 71720f7770..abae0cdace 100644 --- a/src/main/java/org/zalando/nakadi/repository/db/StorageDbRepository.java +++ b/src/main/java/org/zalando/nakadi/repository/db/StorageDbRepository.java @@ -8,11 +8,13 @@ import java.util.Optional; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataAccessException; +import org.springframework.dao.DuplicateKeyException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.stereotype.Repository; import org.zalando.nakadi.annotations.DB; import org.zalando.nakadi.domain.Storage; +import org.zalando.nakadi.exceptions.DuplicatedStorageIdException; import org.zalando.nakadi.exceptions.InternalNakadiException; @DB @@ -48,7 +50,8 @@ public Optional getStorage(final String id) throws InternalNakadiExcept return Optional.ofNullable(storages.isEmpty() ? null : storages.get(0)); } - public Storage createStorage(final Storage storage) throws DataAccessException, InternalNakadiException { + public Storage createStorage(final Storage storage) + throws DataAccessException, DuplicatedStorageIdException, InternalNakadiException { try { jdbcTemplate.update( "INSERT INTO zn_data.storage (st_id, st_type, st_configuration) VALUES (?, ?, ?::jsonb)", @@ -59,6 +62,8 @@ public Storage createStorage(final Storage storage) throws DataAccessException, } catch (final JsonProcessingException ex) { throw new IllegalArgumentException("Storage configuration " + storage.getConfiguration(Object.class) + " can't be mapped to json", ex); + } catch (DuplicateKeyException e) { + throw new DuplicatedStorageIdException("A storage with id '" + storage.getId() + "' already exists.", e); } catch (DataAccessException e) { throw new InternalNakadiException("Error occurred when creating Storage " + storage.getId(), e); } diff --git a/src/main/java/org/zalando/nakadi/service/StorageService.java b/src/main/java/org/zalando/nakadi/service/StorageService.java index 03934b83ca..e46d901391 100644 --- a/src/main/java/org/zalando/nakadi/service/StorageService.java +++ b/src/main/java/org/zalando/nakadi/service/StorageService.java @@ -6,6 +6,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.zalando.nakadi.domain.Storage; +import org.zalando.nakadi.exceptions.DuplicatedStorageIdException; import org.zalando.nakadi.exceptions.InternalNakadiException; import org.zalando.nakadi.repository.db.StorageDbRepository; import org.zalando.problem.Problem; @@ -75,14 +76,6 @@ public Result createStorage(final JSONObject json) { return Result.problem(Problem.valueOf(UNPROCESSABLE_ENTITY, e.getMessage())); } - try { - if (storageDbRepository.getStorage(id).isPresent()) { - return Result.conflict("Storage with ID " + id + " already exists."); - } - } catch (InternalNakadiException e) { - return Result.problem(Problem.valueOf(INTERNAL_SERVER_ERROR, e.getMessage())); - } - final Storage storage = new Storage(); storage.setId(id); storage.setType(Storage.Type.valueOf(type.toUpperCase())); @@ -94,6 +87,8 @@ public Result createStorage(final JSONObject json) { try { storageDbRepository.createStorage(storage); + } catch (DuplicatedStorageIdException e) { + return Result.problem(e.asProblem()); } catch (InternalNakadiException e) { return Result.problem(Problem.valueOf(INTERNAL_SERVER_ERROR, e.getMessage())); } diff --git a/src/test/java/org/zalando/nakadi/service/StorageServiceTest.java b/src/test/java/org/zalando/nakadi/service/StorageServiceTest.java index ece60b0aed..669f348634 100644 --- a/src/test/java/org/zalando/nakadi/service/StorageServiceTest.java +++ b/src/test/java/org/zalando/nakadi/service/StorageServiceTest.java @@ -34,7 +34,6 @@ public void testCreateStorage() throws Exception { final Storage dbReply = createTestStorage(); when(storageDbRepository.createStorage(any())).thenReturn(dbReply); - when(storageDbRepository.getStorage(any())).thenReturn(Optional.empty()); final JSONObject storage = createTestStorageJson("s1"); final Result result = storageService.createStorage(storage);