fix(secret): fix wrong representation of Secret values & add payload #1088
Conversation
|
Many thanks @rschollmeyer. Could you extend the tests accordingly? |
|
Oh right. I'm on it! |
|
Actually there was not much to extend since the changes were mainly changes in representation. |
| @JsonProperty("updated") | ||
| private Date updateTime; | ||
| private String updateTime; |
There was a problem hiding this comment.
Because it is listed as a String in the Barbican API.
See here
There was a problem hiding this comment.
I was asking, because changing the type is a breaking change and it might be more convenient for our Java client to have a Date here instead of a String. But since you're the author I would leave that up to you.
Besides this, this PR is ready to merge from my side.
There was a problem hiding this comment.
Well, i get your point & Date would probably be better. I'll change it back
|
I thought of that too, but the Secret object that is returned when creating seems to only contain the reference. Edit: Forget what i said. I read "createSecret()" instead of "getSecret()". Sure i can do that! |
|
Done the changes. Note that the output of Dates when calling GET for a secret is now "Mon Dec 28 19:17:44 UTC 2015" instead of "2015-12-28T19:14:44.180394" (as specified in the barbican API) |
| @@ -47,14 +48,25 @@ public void testGetSecret() throws IOException { | |||
| Secret secret = osv3().barbican().secrets().get(secretId); | |||
| assertNotNull(secret); | |||
| assertNotNull(secret.getName()); | |||
| assertEquals(secret.getExpiration().toString(), expiration); | |||
There was a problem hiding this comment.
Instead of comparing the strings, I'd suggest to compare the Date types. Make sense? I promise that's the last comment @rschollmeyer.
There was a problem hiding this comment.
Sure i can do that. Don't worry, i am thankful for every good critique!
There was a problem hiding this comment.
To be sure: do you mean to compare the date formats?
There was a problem hiding this comment.
Compare assertEquals(secret.getExpiration(), expiration); where expiration is a Date object
|
Perfect. Thanks for all the work @rschollmeyer 💯 |
Some minor fixes to match the Barbican API.