devicehive / devicehive-java-server

DeviceHive Java Server
http://www.devicehive.com
Apache License 2.0
369 stars 139 forks source link

Device is not assigned to user's default Network #102

Closed demon-xxi closed 9 years ago

demon-xxi commented 9 years ago

When device is added by non admin user with no network specified it is not visible to the user, where it should be assigned to first network available to the user. Is user has no network then 412 Precondition Failed error should returned as no network was found.

Nikolay-Kha commented 9 years ago

+1 for this bug and the fastest fix deploying on PG.

demon-xxi commented 9 years ago

Here's example curl request to create device

curl -X PUT -H "Authorization: Bearer XXXXX_REPLACE_WITH_YOUR_ACCESSKEY" -H "Content-Type: application/json"  -d '{

  "name" : "test-device",
    "deviceClass": {
        "name" : "default device",
         "version": "1.0"
    }
}' 'http://localhost:8080/dh/rest/device/test-device'
Nikolay-Kha commented 9 years ago

@demon-xxi why do you need "key" field? According to the documentation it is optional. I think null keys should be allowed for creating. But DeviceKey authorization with null key shouldn't.

demon-xxi commented 9 years ago

@Nikolay-Kha you are correct, the key is optional, I just copied example from history. Removed to avoid confusion. Thank you for correction.

Nikolay-Kha commented 9 years ago

btw, currently PG returns error without "key" field. "message": "Validation failed for classes [com.devicehive.model.Device] during persist time for groups [javax.validation.groups.Default, ]\nList of constraint violations:[\n\tConstraintViolationImpl{interpolatedMessage='key field cannot be null.', propertyPath=key, rootBeanClass=class com.devicehive.model.Device, messageTemplate='key field cannot be null.'}\n]"

it was in my saturday's email.

demon-xxi commented 9 years ago

yep, in 2.0.6 we switched to a Device class as method parameter instead of generic json. So it has to do something with key field being annotated as required on that class. @zubrabubra, can you make it optional please?

zubrabubra commented 9 years ago

will have a look on key as well as a part of this defect

zubrabubra commented 9 years ago

key was required a long ago

    @SerializedName("key")
    @Column
    @NotNull(message = "key field cannot be null.")
    @Size(min = 1, max = 64, message = "Field cannot be empty. The length of key should not be more than 64 symbols.")
    private String key;

Could cause NPE if we do as a hotfix, while i can spend little bit more and check all usages.

demon-xxi commented 9 years ago

@zubrabubra we should do more checks and run tests for sure but the key is allowed to be null so it's a question how it worked before. Perhaps no key uses an empty string?

demon-xxi commented 9 years ago

This fix was deployed on PG including key not being required now