Servlet api issue 175 add cookie set attribute v3 #401
Conversation
github.com/jakartaee/pull/399#discussion_r624276777
…-setAttribute' into servlet-api-issue-175_add-cookie-setAttribute-v2
|
@stuartwdouglas hopefully this should be transparent change to the TCK, so at least we wont break it and thus it doesn't specifically need a TCK change.... but then it is a change to the API so it should be tested in the TCK... unless we add a unit test in the API project and then add a single new TCK test that runs the unit tests from the project? |
|
Yea, its not that this will break existing stuff, but more that we should be testing that containers implement it properly. Maybe we can just wait until there is more clarity on the TCK development process and add the tests then. |
|
So OK to merge then? |
|
Ah, I just saw the comments on #400 regarding junit5. I will add some unit tests for this now.... |
|
@stuartwdouglas that was worthwhile! Found a few little issues and corner cases that needed to be resolved. Coverage is now 96% |
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| if (MAX_AGE.equalsIgnoreCase(name) && value != null) { | ||
| Long.parseLong(value); |
There was a problem hiding this comment.
Use Integer to be consistent with setMaxAge(int) ?
There was a problem hiding this comment.
I now just call setMaxAge here, rather than duplicate the checking logic.
| <dependency> | ||
| <groupId>org.hamcrest</groupId> | ||
| <artifactId>hamcrest</artifactId> | ||
| <version>2.2</version> |
There was a problem hiding this comment.
Should we move these to a section?
There was a problem hiding this comment.
move the versions to properties?
| http.method_get_not_supported=La m�thode HTTP GET n''est pas support�e par cette URL | ||
| http.method_post_not_supported=La m�thode HTTP POST n''est pas support�e par cette URL | ||
| http.method_put_not_supported=La m�thode HTTP PUT n''est pas support�e par cette URL | ||
| http.method_delete_not_supported=La m�thode HTTP DELETE n''est pas support�e par cette URL |
There was a problem hiding this comment.
Please correct this and fix your editor to not anymore cause this in the future.
Extends #400 with optimisation to lazily create the attribute map.
Also added hash and equality for completeness