libreio / libre

Simple AWS gateway for storing and sharing files
http://libre.io
Other
7 stars 9 forks source link

#301 bit.ly links are broken #303

Closed dmzaytsev closed 9 years ago

dmzaytsev commented 9 years ago

For #301

dmzaytsev commented 9 years ago

@karato travis failure is unrelated to issue. see #299

karato commented 9 years ago

@dmzaytsev I will find a reviewer for your pull requests shortly, thanks for contribution!

karato commented 9 years ago

@krzyk review it

krzyk commented 9 years ago

@dmzaytsev I think that the problem is because @EqualsAndHashcode generated (which is used in cache) will be the default (hashcode = 1, and equals will be always true for object of the same class). This annotation ignores transient fields so we have to add of = {} to it. With that I think it should work without the added seperate cache.

krzyk commented 9 years ago

@dmzaytsev also you have to make sure that all the Doc implementers have correct equals and hashcode (from what I see SmallDoc has the same problem as your implementation, and MkDoc doesn't have any)

dmzaytsev commented 9 years ago

@krzyk I don't know well enough internal implementation jcabi-cache. anyway I will follow to your advice

dmzaytsev commented 9 years ago

@krzyk fixed test passed localy

krzyk commented 9 years ago

@dmzaytsev we will need to wait for #299 to merge

krzyk commented 9 years ago

@karato we are waiting for #299

karato commented 9 years ago

@karato we are waiting for #299

@krzyk yes, right, let's wait for #299

krzyk commented 9 years ago

@dmzaytsev one more comment, please add a conformsToEqualsHashCodeContract in both SmallDocTest and CdShortUrlTest, you can use the one from e.g. AwsUserTest. This way we will know sooner if some fields are not taken into account when doing hascode or equals.

dmzaytsev commented 9 years ago

@krzyk what about other Docs? e.q. SafeDoc

krzyk commented 9 years ago

@dmzaytsev I forgot there are others, yes we need to have it in all the docs.

dmzaytsev commented 9 years ago

@krzyk I added these tests

krzyk commented 9 years ago

@rultor merge pls

rultor commented 9 years ago

@rultor merge pls

@krzyk Thanks for your request. @yegor256 Please confirm this.

yegor256 commented 9 years ago

@rultor merge

rultor commented 9 years ago

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor commented 9 years ago

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 10min)

karato commented 9 years ago

@krzyk 22 mins was added to your account, many thanks for your contribution! The task took 58539002.... review comments (c=7) added as a bonus... +22 to your rating, your total score is +4623

karato commented 9 years ago

@rultor deploy

rultor commented 9 years ago

@rultor deploy

@karato OK, I'll try to deploy now. You can check the progress here

rultor commented 9 years ago

@rultor deploy

@karato Done! FYI, the full log is here (took me 5min)