incorporate feedback (#910)

- ignore fields that are irrelevant - deserialization: use ISO pattern for LocalDateTime - serialization: do not write dates as timestamps. use string and ISO pattern instead - throw 422 "Unprocessable Entity" when trying to create an existing ocpp tag - improve query param defaults - improve API spec documentation
steve-community · Nov 27, 2022 · ba78860 · ba78860
1 parent f364e7a
commit ba78860
Show file tree

Hide file tree

Showing 19 changed files with 257 additions and 46 deletions.
diff --git a/src/main/java/de/rwth/idsg/steve/SteveException.java b/src/main/java/de/rwth/idsg/steve/SteveException.java
@@ -55,4 +55,22 @@ public SteveException(String template, Object arg1, Object arg2) {
     public SteveException(String template, Object arg1, Object arg2, Throwable cause) {
         this(format(template, arg1, arg2), cause);
     }
+
+    // -------------------------------------------------------------------------
+    // Custom/extending classes
+    // -------------------------------------------------------------------------
+
+    public static class AlreadyExists extends SteveException {
+
+        public AlreadyExists(String template, Object arg1) {
+            super(format(template, arg1));
+        }
+    }
+
+    public static class NotFound extends SteveException {
+
+        public NotFound(String message) {
+            super(message);
+        }
+    }
 }
diff --git a/src/main/java/de/rwth/idsg/steve/config/BeanConfiguration.java b/src/main/java/de/rwth/idsg/steve/config/BeanConfiguration.java
@@ -18,6 +18,9 @@
  */
 package de.rwth.idsg.steve.config;
 
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.mysql.cj.conf.PropertyKey;
 import com.zaxxer.hikari.HikariConfig;
@@ -41,6 +44,8 @@
 import org.springframework.context.event.ContextRefreshedEvent;
 import org.springframework.context.event.EventListener;
 import org.springframework.core.Ordered;
+import org.springframework.http.converter.HttpMessageConverter;
+import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
 import org.springframework.scheduling.annotation.EnableScheduling;
 import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
@@ -51,6 +56,7 @@
 
 import javax.annotation.PreDestroy;
 import javax.validation.Validator;
+import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -228,4 +234,23 @@ public void addViewControllers(ViewControllerRegistry registry) {
         registry.setOrder(Ordered.HIGHEST_PRECEDENCE);
     }
 
+    // -------------------------------------------------------------------------
+    // API config
+    // -------------------------------------------------------------------------
+
+    @Override
+    public void extendMessageConverters(List<HttpMessageConverter<?>> converters) {
+        for (HttpMessageConverter<?> converter : converters) {
+            if (converter instanceof MappingJackson2HttpMessageConverter) {
+                MappingJackson2HttpMessageConverter conv = (MappingJackson2HttpMessageConverter) converter;
+                ObjectMapper objectMapper = conv.getObjectMapper();
+                // if the client sends unknown props, just ignore them instead of failing
+                objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
+                // default is true
+                objectMapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
+                break;
+            }
+        }
+    }
+
 }
diff --git a/src/main/java/de/rwth/idsg/steve/repository/dto/OcppTag.java b/src/main/java/de/rwth/idsg/steve/repository/dto/OcppTag.java
@@ -18,6 +18,8 @@
  */
 package de.rwth.idsg.steve.repository.dto;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import io.swagger.annotations.ApiModelProperty;
 import lombok.Builder;
 import lombok.Getter;
 import lombok.ToString;
@@ -43,7 +45,13 @@ public static final class Overview {
         private final boolean inTransaction;
         private final boolean blocked;
 
+        /**
+         * Only relevant for the web pages. Disabled for API
+         */
+        @JsonIgnore
+        @ApiModelProperty(hidden = true)
         private final String expiryDateFormatted;
+
         private final DateTime expiryDate;
 
         private final Integer maxActiveTransactionCount;

diff --git a/src/main/java/de/rwth/idsg/steve/repository/dto/Transaction.java b/src/main/java/de/rwth/idsg/steve/repository/dto/Transaction.java
@@ -18,6 +18,8 @@
  */
 package de.rwth.idsg.steve.repository.dto;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import io.swagger.annotations.ApiModelProperty;
 import jooq.steve.db.enums.TransactionStopEventActor;
 import lombok.Builder;
 import lombok.Getter;
@@ -34,13 +36,44 @@
 @Builder
 @ToString
 public final class Transaction {
-    private final int id, connectorId, chargeBoxPk, ocppTagPk;
-    private final String chargeBoxId, ocppIdTag, startTimestampFormatted, startValue;
+
+    private final int id;
+    private final int connectorId;
+    private final int chargeBoxPk;
+    private final int ocppTagPk;
+
+    private final String chargeBoxId;
+
+    private final String ocppIdTag;
+
+    /**
+     * Only relevant for the web pages. Disabled for API
+     */
+    @JsonIgnore
+    @ApiModelProperty(hidden = true)
+    private final String startTimestampFormatted;
+
+    private final String startValue;
+
     private final DateTime startTimestamp;
 
-    @Nullable private final String stopTimestampFormatted;
-    @Nullable private final String stopValue;
-    @Nullable private final String stopReason; // new in OCPP 1.6
-    @Nullable private final DateTime stopTimestamp;
-    @Nullable private final TransactionStopEventActor stopEventActor;
+    /**
+     * Only relevant for the web pages. Disabled for API
+     */
+    @JsonIgnore
+    @ApiModelProperty(hidden = true)
+    @Nullable
+    private final String stopTimestampFormatted;
+
+    @Nullable
+    private final String stopValue;
+
+    @Nullable
+    private final String stopReason; // new in OCPP 1.6
+
+    @Nullable
+    private final DateTime stopTimestamp;
+
+    @Nullable
+    private final TransactionStopEventActor stopEventActor;
 }
diff --git a/src/main/java/de/rwth/idsg/steve/repository/impl/OcppTagRepositoryImpl.java b/src/main/java/de/rwth/idsg/steve/repository/impl/OcppTagRepositoryImpl.java
@@ -209,7 +209,7 @@ public int addOcppTag(OcppTagForm u) {
 
         } catch (DataAccessException e) {
             if (e.getCause() instanceof SQLIntegrityConstraintViolationException) {
-                throw new SteveException("A user with idTag '%s' already exists.", u.getIdTag());
+                throw new SteveException.AlreadyExists("A user with idTag '%s' already exists.", u.getIdTag());
             } else {
                 throw new SteveException("Execution of addOcppTag for idTag '%s' FAILED.", u.getIdTag(), e);
             }

diff --git a/src/main/java/de/rwth/idsg/steve/web/GlobalControllerAdvice.java b/src/main/java/de/rwth/idsg/steve/web/GlobalControllerAdvice.java
@@ -46,7 +46,7 @@ public void binder(WebDataBinder binder) {
 
         binder.registerCustomEditor(String.class, new StringTrimmerEditor(true));
         binder.registerCustomEditor(LocalDate.class, new LocalDateEditor());
-        binder.registerCustomEditor(LocalDateTime.class, new LocalDateTimeEditor());
+        binder.registerCustomEditor(LocalDateTime.class, LocalDateTimeEditor.forMvc());
         binder.registerCustomEditor(ChargePointSelect.class, new ChargePointSelectEditor());
 
         binder.registerCustomEditor(List.class, "idList", batchInsertConverter);

diff --git a/src/main/java/de/rwth/idsg/steve/web/LocalDateTimeEditor.java b/src/main/java/de/rwth/idsg/steve/web/LocalDateTimeEditor.java
@@ -19,27 +19,40 @@
 package de.rwth.idsg.steve.web;
 
 import com.google.common.base.Strings;
+import lombok.AccessLevel;
+import lombok.RequiredArgsConstructor;
 import org.joda.time.LocalDateTime;
 import org.joda.time.format.DateTimeFormat;
 import org.joda.time.format.DateTimeFormatter;
+import org.joda.time.format.ISODateTimeFormat;
+import org.springframework.security.core.parameters.P;
 
 import java.beans.PropertyEditorSupport;
 
 /**
  * @author Sevket Goekay <sevketgokay@gmail.com>
  * @since 04.01.2015
  */
+@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
 public class LocalDateTimeEditor extends PropertyEditorSupport {
 
-    private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormat.forPattern("yyyy-MM-dd HH:mm");
+    private final DateTimeFormatter dateTimeFormatter;
+
+    public static LocalDateTimeEditor forMvc() {
+        return new LocalDateTimeEditor(DateTimeFormat.forPattern("yyyy-MM-dd HH:mm"));
+    }
+
+    public static LocalDateTimeEditor forApi() {
+        return new LocalDateTimeEditor(ISODateTimeFormat.localDateOptionalTimeParser());
+    }
 
     @Override
     public String getAsText() {
         Object value = getValue();
         if (value == null) {
             return null;
         } else {
-            return DATE_TIME_FORMATTER.print((LocalDateTime) value);
+            return dateTimeFormatter.print((LocalDateTime) value);
         }
     }
 
@@ -48,7 +61,7 @@ public void setAsText(String text) {
         if (Strings.isNullOrEmpty(text)) {
             setValue(null);
         } else {
-            setValue(DATE_TIME_FORMATTER.parseLocalDateTime(text));
+            setValue(dateTimeFormatter.parseLocalDateTime(text));
         }
     }
 }
diff --git a/src/main/java/de/rwth/idsg/steve/web/api/ApiControllerAdvice.java b/src/main/java/de/rwth/idsg/steve/web/api/ApiControllerAdvice.java
@@ -18,9 +18,9 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.web.LocalDateTimeEditor;
 import de.rwth.idsg.steve.web.api.exception.BadRequestException;
-import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
 import org.joda.time.DateTime;
@@ -48,7 +48,7 @@ public class ApiControllerAdvice {
     @InitBinder
     public void binder(WebDataBinder binder) {
         binder.registerCustomEditor(String.class, new StringTrimmerEditor(true));
-        binder.registerCustomEditor(LocalDateTime.class, new LocalDateTimeEditor());
+        binder.registerCustomEditor(LocalDateTime.class, LocalDateTimeEditor.forApi());
     }
 
     @ExceptionHandler(BindException.class)
@@ -59,14 +59,22 @@ public ApiErrorResponse handleBindException(HttpServletRequest req, BindExceptio
         return createResponse(url, HttpStatus.BAD_REQUEST, "Error understanding the request");
     }
 
-    @ExceptionHandler(NotFoundException.class)
+    @ExceptionHandler(SteveException.NotFound.class)
     @ResponseStatus(value = HttpStatus.NOT_FOUND)
-    public ApiErrorResponse handleNotFoundException(HttpServletRequest req, NotFoundException exception) {
+    public ApiErrorResponse handleNotFoundException(HttpServletRequest req, SteveException.NotFound exception) {
         String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
         return createResponse(url, HttpStatus.NOT_FOUND, exception.getMessage());
     }
 
+    @ExceptionHandler(SteveException.AlreadyExists.class)
+    @ResponseStatus(value = HttpStatus.UNPROCESSABLE_ENTITY)
+    public ApiErrorResponse handleAlreadyExistsException(HttpServletRequest req, SteveException.AlreadyExists exception) {
+        String url = req.getRequestURL().toString();
+        log.error("Request: {} raised following exception.", url, exception);
+        return createResponse(url, HttpStatus.UNPROCESSABLE_ENTITY, exception.getMessage());
+    }
+
     @ExceptionHandler(BadRequestException.class)
     @ResponseStatus(value = HttpStatus.BAD_REQUEST)
     public ApiErrorResponse handleBadRequestException(HttpServletRequest req, BadRequestException exception) {
@@ -94,7 +102,7 @@ public ApiErrorResponse handleException(HttpServletRequest req, Exception except
     public static ApiErrorResponse createResponse(String url, HttpStatus status, String message) {
         ApiErrorResponse result = new ApiErrorResponse();
 
-        result.setTimestamp(DateTime.now().toString());
+        result.setTimestamp(DateTime.now());
         result.setStatus(status.value());
         result.setError(status.getReasonPhrase());
         result.setMessage(message);
@@ -105,7 +113,7 @@ public static ApiErrorResponse createResponse(String url, HttpStatus status, Str
 
     @Data
     public static class ApiErrorResponse {
-        private String timestamp;
+        private DateTime timestamp;
         private int status;
         private String error;
         private String message;

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/OcppTagsRestController.java b/src/main/java/de/rwth/idsg/steve/web/api/OcppTagsRestController.java
@@ -18,11 +18,11 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.repository.OcppTagRepository;
 import de.rwth.idsg.steve.repository.dto.OcppTag;
 import de.rwth.idsg.steve.service.OcppTagService;
 import de.rwth.idsg.steve.web.api.ApiControllerAdvice.ApiErrorResponse;
-import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import de.rwth.idsg.steve.web.dto.OcppTagForm;
 import de.rwth.idsg.steve.web.dto.OcppTagQueryForm;
 import io.swagger.annotations.ApiResponse;
@@ -67,7 +67,7 @@ public class OcppTagsRestController {
     )
     @GetMapping(value = "")
     @ResponseBody
-    public List<OcppTag.Overview> get(OcppTagQueryForm params) {
+    public List<OcppTag.Overview> get(OcppTagQueryForm.ForApi params) {
         log.debug("Read request for query: {}", params);
 
         var response = ocppTagRepository.getOverview(params);
@@ -96,6 +96,7 @@ public OcppTag.Overview getOne(@PathVariable("ocppTagPk") Integer ocppTagPk) {
         @ApiResponse(code = 201, message = "Created"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
+        @ApiResponse(code = 422, message = "Unprocessable Entity", response = ApiErrorResponse.class),
         @ApiResponse(code = 404, message = "Not Found", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
@@ -153,12 +154,12 @@ public OcppTag.Overview delete(@PathVariable("ocppTagPk") Integer ocppTagPk) {
     }
 
     private OcppTag.Overview getOneInternal(int ocppTagPk) {
-        OcppTagQueryForm params = new OcppTagQueryForm();
+        OcppTagQueryForm.ForApi params = new OcppTagQueryForm.ForApi();
         params.setOcppTagPk(ocppTagPk);
 
         List<OcppTag.Overview> results = ocppTagRepository.getOverview(params);
         if (results.isEmpty()) {
-            throw new NotFoundException("Could not find this ocppTag");
+            throw new SteveException.NotFound("Could not find this ocppTag");
         }
         return results.get(0);
     }

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java b/src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java
@@ -56,7 +56,7 @@ public class TransactionsRestController {
     )
     @GetMapping(value = "")
     @ResponseBody
-    public List<Transaction> get(@Valid TransactionQueryForm params) {
+    public List<Transaction> get(@Valid TransactionQueryForm.ForApi params) {
         log.debug("Read request for query: {}", params);
 
         if (params.isReturnCSV()) {

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/exception/BadRequestException.java b/src/main/java/de/rwth/idsg/steve/web/api/exception/BadRequestException.java
@@ -1,3 +1,21 @@
+/*
+ * SteVe - SteckdosenVerwaltung - https://github.com/steve-community/steve
+ * Copyright (C) 2013-2019 RWTH Aachen University - Information Systems - Intelligent Distributed Systems Group (IDSG).
+ * All Rights Reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
 package de.rwth.idsg.steve.web.api.exception;
 
 public class BadRequestException extends RuntimeException {

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/exception/NotFoundException.java b/src/main/java/de/rwth/idsg/steve/web/api/exception/NotFoundException.java