Merge pull request #933 from zalando/refactor-exceptions

ARUHA-1887 Refactor exceptions
zalando · Sep 4, 2018 · e6accdd · e6accdd
2 parents 0fdb7ed + 99bdd9d
commit e6accdd
Show file tree

Hide file tree

Showing 189 changed files with 672 additions and 891 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 - Upgraded dependencies
+- Refactored exceptions
 
 ## [2.8.3] - 2018-08-01
 

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -45,7 +45,7 @@ services:
       - "2181:2181"
 
   kafka:
-    image: wurstmeister/kafka:1.1.0
+    image: wurstmeister/kafka:2.11-1.1.1
     network_mode: "host"
     ports:
       - "9092:9092"

diff --git a/src/acceptance-test/java/org/zalando/nakadi/repository/db/EventTypeDbRepositoryTest.java b/src/acceptance-test/java/org/zalando/nakadi/repository/db/EventTypeDbRepositoryTest.java
@@ -9,9 +9,8 @@
 import org.zalando.nakadi.domain.EventType;
 import org.zalando.nakadi.domain.EventTypeSchema;
 import org.zalando.nakadi.domain.Version;
-import org.zalando.nakadi.exceptions.NakadiException;
-import org.zalando.nakadi.exceptions.NoSuchEventTypeException;
 import org.zalando.nakadi.exceptions.runtime.DuplicatedEventTypeNameException;
+import org.zalando.nakadi.exceptions.runtime.NoSuchEventTypeException;
 import org.zalando.nakadi.repository.EventTypeRepository;
 import org.zalando.nakadi.utils.TestUtils;
 
@@ -100,12 +99,12 @@ public void whenEventExistsFindByNameReturnsSomething() throws Exception {
     }
 
     @Test(expected = NoSuchEventTypeException.class)
-    public void whenEventDoesntExistsFindByNameReturnsNothing() throws NakadiException {
+    public void whenEventDoesntExistsFindByNameReturnsNothing() {
         repository.findByName("inexisting-name");
     }
 
     @Test
-    public void whenUpdateExistingEventTypeItUpdates() throws NakadiException, IOException {
+    public void whenUpdateExistingEventTypeItUpdates() throws IOException {
         final EventType eventType = buildDefaultEventType();
 
         repository.saveEventType(eventType);
@@ -135,7 +134,7 @@ public void whenUpdateExistingEventTypeItUpdates() throws NakadiException, IOExc
     }
 
     @Test
-    public void whenUpdateDifferentSchemaVersionThenInsertIt() throws NakadiException, IOException {
+    public void whenUpdateDifferentSchemaVersionThenInsertIt() throws IOException {
         final EventType eventType = buildDefaultEventType();
 
         repository.saveEventType(eventType);
@@ -151,7 +150,7 @@ public void whenUpdateDifferentSchemaVersionThenInsertIt() throws NakadiExceptio
     }
 
     @Test
-    public void whenListExistingEventTypesAreListed() throws NakadiException {
+    public void whenListExistingEventTypesAreListed() {
         final EventType eventType1 = buildDefaultEventType();
         final EventType eventType2 = buildDefaultEventType();
 

diff --git a/src/acceptance-test/java/org/zalando/nakadi/repository/db/StorageDbRepositoryTest.java b/src/acceptance-test/java/org/zalando/nakadi/repository/db/StorageDbRepositoryTest.java
@@ -5,7 +5,7 @@
 import org.zalando.nakadi.domain.EventType;
 import org.zalando.nakadi.domain.Storage;
 import org.zalando.nakadi.domain.Timeline;
-import org.zalando.nakadi.exceptions.runtime.NoStorageException;
+import org.zalando.nakadi.exceptions.runtime.NoSuchStorageException;
 import org.zalando.nakadi.exceptions.runtime.StorageIsUsedException;
 import org.zalando.nakadi.utils.TestUtils;
 
@@ -94,7 +94,7 @@ public void testStorageDeleted() throws Exception {
         assertFalse(repository.getStorage(storage.getId()).isPresent());
     }
 
-    @Test(expected = NoStorageException.class)
+    @Test(expected = NoSuchStorageException.class)
     public void testDeleteNoneExistingStorage() throws Exception {
         repository.deleteStorage(randomUUID());
     }

diff --git a/src/acceptance-test/java/org/zalando/nakadi/repository/db/SubscriptionDbRepositoryTest.java b/src/acceptance-test/java/org/zalando/nakadi/repository/db/SubscriptionDbRepositoryTest.java
@@ -8,7 +8,7 @@
 import org.zalando.nakadi.config.JsonConfig;
 import org.zalando.nakadi.domain.Subscription;
 import org.zalando.nakadi.domain.SubscriptionBase;
-import org.zalando.nakadi.exceptions.NoSuchSubscriptionException;
+import org.zalando.nakadi.exceptions.runtime.NoSuchSubscriptionException;
 import org.zalando.nakadi.exceptions.runtime.DuplicatedSubscriptionException;
 import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException;
 import org.zalando.nakadi.util.HashGenerator;

diff --git a/src/acceptance-test/java/org/zalando/nakadi/webservice/EventStreamReadingAT.java b/src/acceptance-test/java/org/zalando/nakadi/webservice/EventStreamReadingAT.java
@@ -262,7 +262,7 @@ public void whenReachKeepAliveLimitThenStreamIsClosed() {
     }
 
     @Test(timeout = 5000)
-    public void whenGetEventsWithUknownTopicThenTopicNotFound() {
+    public void whenGetEventsWithUnknownTopicThenTopicNotFound() {
         given()
                 .when()
                 .get(createStreamEndpointUrl("blah-topic"))

diff --git a/src/acceptance-test/java/org/zalando/nakadi/webservice/EventTypeAT.java b/src/acceptance-test/java/org/zalando/nakadi/webservice/EventTypeAT.java
@@ -18,7 +18,7 @@
 import org.zalando.nakadi.domain.ResourceAuthorization;
 import org.zalando.nakadi.domain.ResourceAuthorizationAttribute;
 import org.zalando.nakadi.domain.Timeline;
-import org.zalando.nakadi.exceptions.NoSuchEventTypeException;
+import org.zalando.nakadi.exceptions.runtime.NoSuchEventTypeException;
 import org.zalando.nakadi.partitioning.PartitionStrategy;
 import org.zalando.nakadi.repository.kafka.KafkaTestHelper;
 import org.zalando.nakadi.utils.EventTypeTestBuilder;

diff --git a/src/acceptance-test/java/org/zalando/nakadi/webservice/PartitionsControllerAT.java b/src/acceptance-test/java/org/zalando/nakadi/webservice/PartitionsControllerAT.java
@@ -11,7 +11,7 @@
 import org.junit.Test;
 import org.springframework.http.HttpStatus;
 import org.zalando.nakadi.domain.EventType;
-import org.zalando.nakadi.exceptions.NoSuchEventTypeException;
+import org.zalando.nakadi.exceptions.runtime.NoSuchEventTypeException;
 import org.zalando.nakadi.repository.kafka.KafkaTestHelper;
 import org.zalando.nakadi.utils.EventTypeTestBuilder;
 import org.zalando.nakadi.view.Cursor;

diff --git a/src/main/java/org/zalando/nakadi/config/NakadiConfig.java b/src/main/java/org/zalando/nakadi/config/NakadiConfig.java
@@ -14,7 +14,7 @@
 import org.springframework.scheduling.annotation.EnableScheduling;
 import org.zalando.nakadi.domain.DefaultStorage;
 import org.zalando.nakadi.domain.Storage;
-import org.zalando.nakadi.exceptions.InternalNakadiException;
+import org.zalando.nakadi.exceptions.runtime.InternalNakadiException;
 import org.zalando.nakadi.exceptions.runtime.DuplicatedStorageException;
 import org.zalando.nakadi.repository.db.StorageDbRepository;
 import org.zalando.nakadi.repository.zookeeper.ZooKeeperHolder;

diff --git a/src/main/java/org/zalando/nakadi/config/SecurityConfiguration.java b/src/main/java/org/zalando/nakadi/config/SecurityConfiguration.java
@@ -4,7 +4,6 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
-import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.converter.HttpMessageConverter;
@@ -20,22 +19,13 @@
 import org.springframework.security.oauth2.provider.error.OAuth2AccessDeniedHandler;
 import org.springframework.security.oauth2.provider.error.OAuth2AuthenticationEntryPoint;
 import org.springframework.security.oauth2.provider.token.ResourceServerTokenServices;
-import org.springframework.security.web.firewall.FirewalledRequest;
-import org.springframework.security.web.firewall.HttpFirewall;
-import org.springframework.security.web.firewall.RequestRejectedException;
-import org.springframework.security.web.firewall.StrictHttpFirewall;
 import org.zalando.stups.oauth2.spring.security.expression.ExtendedOAuth2WebSecurityExpressionHandler;
 
-import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.Response;
 import java.io.IOException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import static org.springframework.http.HttpMethod.DELETE;
 import static org.springframework.http.HttpMethod.GET;
@@ -199,141 +189,4 @@ public String getDetail() {
         }
     }
 
-    // TODO: REMOVE IT AFTER EVERYONE HAS NORMALIZED THEIR URLS
-    @Bean
-    public HttpFirewall allowUrlEncodedSlashHttpFirewall() {
-        return new AllowForwardSlashesStrictHttpFirewall();
-    }
-
-    // TODO: REMOVE IT AFTER EVERYONE HAS NORMALIZED THEIR URLS
-    private static class AllowForwardSlashesStrictHttpFirewall extends StrictHttpFirewall {
-
-        private static final String ENCODED_PERCENT = "%25";
-        private static final String PERCENT = "%";
-        private static final List<String> FORBIDDEN_ENCODED_PERIOD =
-                Collections.unmodifiableList(Arrays.asList("%2e", "%2E"));
-        private Set<String> encodedUrlBlacklist = new HashSet<>();
-        private Set<String> decodedUrlBlacklist = new HashSet<>();
-
-        AllowForwardSlashesStrictHttpFirewall() {
-            super();
-            this.encodedUrlBlacklist.add(ENCODED_PERCENT);
-            this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD);
-            this.decodedUrlBlacklist.add(PERCENT);
-        }
-
-        private static boolean containsOnlyPrintableAsciiCharacters(final String uri) {
-            final int length = uri.length();
-            for (int i = 0; i < length; i++) {
-                final char c = uri.charAt(i);
-                if (c < '\u0020' || c > '\u007e') {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-
-        private static boolean encodedUrlContains(final HttpServletRequest request, final String value) {
-            if (valueContains(request.getContextPath(), value)) {
-                return true;
-            }
-            return valueContains(request.getRequestURI(), value);
-        }
-
-        private static boolean decodedUrlContains(final HttpServletRequest request, final String value) {
-            if (valueContains(request.getServletPath(), value)) {
-                return true;
-            }
-            if (valueContains(request.getPathInfo(), value)) {
-                return true;
-            }
-            return false;
-        }
-
-        private static boolean valueContains(final String value, final String contains) {
-            return value != null && value.contains(contains);
-        }
-
-        @Override
-        public FirewalledRequest getFirewalledRequest(final HttpServletRequest request)
-                throws RequestRejectedException {
-            rejectedBlacklistedUrls(request);
-
-            if (!isNormalized(request)) {
-                throw new RequestRejectedException("The request was rejected because the URL was not normalized.");
-            }
-
-            final String requestUri = request.getRequestURI();
-            if (!containsOnlyPrintableAsciiCharacters(requestUri)) {
-                throw new RequestRejectedException("The requestURI was rejected because it can only " +
-                        "contain printable ASCII characters.");
-            }
-            return new FirewalledRequest(request) {
-                @Override
-                public void reset() {
-                }
-            };
-        }
-
-        private static boolean isNormalized(final HttpServletRequest request) {
-            if (!isNormalized(request.getRequestURI())) {
-                return false;
-            }
-            if (!isNormalized(request.getContextPath())) {
-                return false;
-            }
-            if (!isNormalized(request.getServletPath())) {
-                return false;
-            }
-            if (!isNormalized(request.getPathInfo())) {
-                return false;
-            }
-            return true;
-        }
-
-        private static boolean isNormalized(final String path) {
-            if (path == null) {
-                return true;
-            }
-
-            // ONLY THIS PART IS REMOVED, ALL OTHER CODE IS THE SAME AS IN StrictHttpFirewall
-            // if (path.indexOf("//") > -1) {
-            //     return false;
-            // }
-
-            for (int j = path.length(); j > 0;) {
-                final int i = path.lastIndexOf('/', j - 1);
-                final int gap = j - i;
-
-                if (gap == 2 && path.charAt(i + 1) == '.') {
-                    // ".", "/./" or "/."
-                    return false;
-                } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') {
-                    return false;
-                }
-
-                j = i;
-            }
-
-            return true;
-        }
-
-        private void rejectedBlacklistedUrls(final HttpServletRequest request) {
-            for (final String forbidden : this.encodedUrlBlacklist) {
-                if (encodedUrlContains(request, forbidden)) {
-                    throw new RequestRejectedException("The request was rejected because the URL contained " +
-                            "a potentially malicious String \"" + forbidden + "\"");
-                }
-            }
-            for (final String forbidden : this.decodedUrlBlacklist) {
-                if (decodedUrlContains(request, forbidden)) {
-                    throw new RequestRejectedException("The request was rejected because the URL contained " +
-                            "a potentially malicious String \"" + forbidden + "\"");
-                }
-            }
-        }
-
-    }
-
 }
diff --git a/src/main/java/org/zalando/nakadi/controller/CursorOperationsController.java b/src/main/java/org/zalando/nakadi/controller/CursorOperationsController.java
@@ -15,14 +15,13 @@
 import org.zalando.nakadi.domain.NakadiCursor;
 import org.zalando.nakadi.domain.NakadiCursorLag;
 import org.zalando.nakadi.domain.ShiftedNakadiCursor;
-import org.zalando.nakadi.exceptions.InternalNakadiException;
-import org.zalando.nakadi.exceptions.InvalidCursorException;
-import org.zalando.nakadi.exceptions.NakadiException;
-import org.zalando.nakadi.exceptions.NoSuchEventTypeException;
-import org.zalando.nakadi.exceptions.runtime.NotFoundException;
 import org.zalando.nakadi.exceptions.runtime.CursorConversionException;
+import org.zalando.nakadi.exceptions.runtime.InternalNakadiException;
+import org.zalando.nakadi.exceptions.runtime.InvalidCursorException;
 import org.zalando.nakadi.exceptions.runtime.InvalidCursorOperation;
-import org.zalando.nakadi.exceptions.runtime.MyNakadiRuntimeException1;
+import org.zalando.nakadi.exceptions.runtime.NakadiBaseException;
+import org.zalando.nakadi.exceptions.runtime.NoSuchEventTypeException;
+import org.zalando.nakadi.exceptions.runtime.NotFoundException;
 import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException;
 import org.zalando.nakadi.repository.EventTypeRepository;
 import org.zalando.nakadi.service.AuthorizationValidator;
@@ -83,7 +82,7 @@ public ResponseEntity<?> getDistance(@PathVariable("eventTypeName") final String
                 final Long distance = cursorOperationsService.calculateDistance(initialCursor, finalCursor);
                 query.setDistance(distance);
             } catch (InternalNakadiException | ServiceTemporarilyUnavailableException e) {
-                throw new MyNakadiRuntimeException1("problem calculating cursors distance", e);
+                throw new NakadiBaseException("problem calculating cursors distance", e);
             } catch (final NoSuchEventTypeException e) {
                 throw new NotFoundException("event type not found", e);
             } catch (final InvalidCursorException e) {
@@ -151,7 +150,7 @@ private String clientErrorMessage(final InvalidCursorOperation.Reason reason) {
                     "have matching partitions.";
             default:
                 LOG.error("Unexpected invalid cursor operation reason " + reason);
-                throw new MyNakadiRuntimeException1();
+                throw new NakadiBaseException();
         }
     }
 
@@ -168,7 +167,7 @@ private Function<Cursor, NakadiCursor> toNakadiCursor(final String eventTypeName
         return cursor -> {
             try {
                 return cursorConverter.convert(eventTypeName, cursor);
-            } catch (final NakadiException | InvalidCursorException e) {
+            } catch (final InternalNakadiException | InvalidCursorException e) {
                 throw new CursorConversionException("problem converting cursors", e);
             }
         };
@@ -180,7 +179,7 @@ private Function<ShiftedCursor, ShiftedNakadiCursor> toShiftedNakadiCursor(final
                 final NakadiCursor nakadiCursor = cursorConverter.convert(eventTypeName, cursor);
                 return new ShiftedNakadiCursor(nakadiCursor.getTimeline(), nakadiCursor.getPartition(),
                         nakadiCursor.getOffset(), cursor.getShift());
-            } catch (final NakadiException | InvalidCursorException e) {
+            } catch (final InternalNakadiException | InvalidCursorException e) {
                 throw new CursorConversionException("problem converting cursors", e);
             }
         };