AeroGear Push
  1. AeroGear Push
  2. AGPUSH-255

Add server side validation and appropriate response body for bad requests

    Details

    • Similar Issues:
      Show 10 results 

      Description

      Add server side validation to avoid persistence exceptions.

      eg: Trying to register an installation with long name results to javax.persistence.PersistenceException: org.hibernate.exception.DataException: Value too long for column "DEVICETOKEN VARCHAR(255) SELECTIVITY 100"

      This happens for almost all the basic operations like Push App registration, Variant registration and installation registrations.

      In addition the WS responses do not contain information about the bad request causes. So in order to order to understand what went wrong, someone has to check the server side logs.

      e.g:
      if (androidVariant.getGoogleKey() == null)

      { return Response.status(Status.BAD_REQUEST).build(); }

      It would be a better approach to remove snippets like the above one, to add Bean Validation checks and fill the response entity with a Map of ConstraintViolation property paths (bean root class) and messages.

      If you believe that this issue is better described as a feature request, feel free to modify it.

        Activity

        Hide
        Matthias Wessendorf
        added a comment -

        Tolis Emmanouilidis do you have the CURL for that handy ?

        Show
        Matthias Wessendorf
        added a comment - Tolis Emmanouilidis do you have the CURL for that handy ?
        Hide
        Tolis Emmanouilidis
        added a comment - - edited

        Matthias Wessendorf This CURL:

        curl -v -b cookies.txt -c cookies.txt -v -H "Accept: application/json" -H "Content-type: application/json" -X POST -d '

        {"name" : "MyApppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp", "description" : "awesome app" }

        ' http://127.0.0.1:8080/ag-push/rest/applications

        produces:

        Caused by: org.h2.jdbc.JdbcSQLException: Value too long for column "NAME VARCHAR(255) NOT NULL": "'MyApppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp... (537)"; SQL statement:
        insert into PushApplication (description, developer, masterSecret, name, pushApplicationID, id) values (?, ?, ?, ?, ?, ?) [22001-161]

        Show
        Tolis Emmanouilidis
        added a comment - - edited Matthias Wessendorf This CURL: curl -v -b cookies.txt -c cookies.txt -v -H "Accept: application/json" -H "Content-type: application/json" -X POST -d ' {"name" : "MyApppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp", "description" : "awesome app" } ' http://127.0.0.1:8080/ag-push/rest/applications produces: Caused by: org.h2.jdbc.JdbcSQLException: Value too long for column "NAME VARCHAR(255) NOT NULL": "'MyApppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp... (537)"; SQL statement: insert into PushApplication (description, developer, masterSecret, name, pushApplicationID, id) values (?, ?, ?, ?, ?, ?) [22001-161]
        Hide
        Matthias Wessendorf
        added a comment -

        Tolis Emmanouilidis the Bean Validation checks are already in place:
        https://github.com/aerogear/aerogear-unifiedpush-server/blob/master/src/main/java/org/jboss/aerogear/unifiedpush/model/AndroidVariant.java#L40-L42

        The 'manual' checks where added, so that we don't even need to bug the JPA layer; however we can look into your suggestion.

        BTW. not sure how exactly I can fill the response entity with a Map of ConstraintViolation property paths (bean root class) and messages.

        Show
        Matthias Wessendorf
        added a comment - Tolis Emmanouilidis the Bean Validation checks are already in place: https://github.com/aerogear/aerogear-unifiedpush-server/blob/master/src/main/java/org/jboss/aerogear/unifiedpush/model/AndroidVariant.java#L40-L42 The 'manual' checks where added, so that we don't even need to bug the JPA layer; however we can look into your suggestion. BTW. not sure how exactly I can fill the response entity with a Map of ConstraintViolation property paths (bean root class) and messages.
        Hide
        Matthias Wessendorf
        added a comment -
        Show
        Matthias Wessendorf
        added a comment - Tolis Emmanouilidis found it
        Hide
        Tolis Emmanouilidis
        added a comment -

        Sorry, I didn't express it well I was thinking that using javax.validation.Validator would be a cleaner way to validate the bean instances instead of performing manual checks (like the null check mentioned above)

        e.g:

        @Inject
        private Validator validator;

        Set<ConstraintViolation<AndroidVariant>> violations = validator.validate(androidVariantInstance);

        The above code validates your instance according the BV rules (@NotNull etc) and returns a Set of javax.validation.ConstraintViolation.

        Show
        Tolis Emmanouilidis
        added a comment - Sorry, I didn't express it well I was thinking that using javax.validation.Validator would be a cleaner way to validate the bean instances instead of performing manual checks (like the null check mentioned above) e.g: @Inject private Validator validator; Set<ConstraintViolation<AndroidVariant>> violations = validator.validate(androidVariantInstance); The above code validates your instance according the BV rules (@NotNull etc) and returns a Set of javax.validation.ConstraintViolation.

          People

          • Assignee:
            Matthias Wessendorf
            Reporter:
            Tolis Emmanouilidis
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: