feat: add Browser.cookies with a CookieJar - #81
Conversation
Fills in the CookieJar that Browser and DefaultBrowser already had commented out, porting Zendriver's get_all/set_all/save/load/clear. Three deliberate departures from the original: Cookies are read and written over the browser connection rather than by picking the first non-closed tab. They live at browser level, so the tab is irrelevant, and `closed` is private to DefaultConnection anyway. Sessions are stored as JSON rather than pickle, which has no multiplatform equivalent. kotlinx.serialization already covers the generated Network.Cookie, and the file stays readable. The `pattern` argument now actually filters what gets written. Upstream builds `included_cookies` and then dumps `cookies`, so the pattern is computed and dropped; a test covers this. save() and load() also return the cookies they selected, so callers can see what a pattern matched instead of guessing. Loading converts each Cookie back into a CookieParam. `size` and `session` are dropped on purpose: both are derived by the browser, and `session` is just the absence of an expiry, which CookieParam.expires already carries. Fixes cdpdriver#27
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Follow-up on the |
… race The four cookie tests each navigated to example.com, which the Storage domain does not need: cookies are browser-scoped and setAll carries the domain itself. Dropping it removes four page loads from BrowserTest. testUpdateTargetSetsTargetTitle read the target title straight after get() with nothing waiting for the load, so it could observe about:blank. It has failed on main before; adding the cookie tests to the same class was enough to tip it over on the Windows runner. It now waits for the ready state first.
|
Pushed a follow-up for the Windows failure. It was Two changes:
Happy to drop the second one if you would rather keep it out of this PR. |
Both were flagged: detekt wants documentation on the companion, and the two settings sat on one line separated by a semicolon.
Fixes #27.
Implements
Browser.cookiesand theCookieJarbehind it, filling in the scaffolding thatBrowserandDefaultBrowseralready carried in comments.CookieJaris an interface with aDefaultCookieJarimplementation, matching howTab/DefaultTabandBrowser/DefaultBrowserare already split here.OpenTelemetryBrowserpicks it up for free through itsby browserdelegation.Where this departs from the Zendriver original, and why
Cookies go over the browser connection, not through a tab. The original walks
browser.tabs, takes the first non-closed one and falls back tobrowser.connection. Cookies are browser-scoped through theStoragedomain, so the tab makes no difference to the result. It also cannot be expressed here:closedis private toDefaultConnectionand not on theConnectioninterface. Usingbrowser.connectiondirectly is both simpler and equivalent.Sessions are JSON, not pickle.
picklehas no multiplatform equivalent, andNetwork.Cookieis already@Serializable, sokotlinx.serializationdoes the job and leaves a file you can actually read.patternnow filters what is written. This is a real bug in the code being ported:included_cookiesis built and then discarded, sosave(pattern=...)always wrote every cookie.testSavePatternOnlyKeepsMatchingCookiescovers it. The pattern is matched against the serialized cookie, which is the closest well-defined equivalent to the original'sstr(cookie.__dict__).saveandloadreturn the cookies they selected, so a caller can see what a pattern actually matched rather than reopening the file to find out.requests_cookie_formatis not ported, being specific to the Pythonrequestslibrary.Restoring cookies
getAllyieldsNetwork.CookiewhilesetAlltakesNetwork.CookieParam, soloadconverts between them through an internalCookie.toParam().sizeandsessionare dropped deliberately: both are derived by the browser, andsessionis merely the absence of an expiry, whichCookieParam.expiresalready expresses.Tests
Four integration tests in
BrowserTest, against a real headless Chrome:setAllthengetAllround trip, checking name, value and domain;clearempties the jar;savethenclearthenloadrestores the cookie, which is the actual use case of the pair;savewith a pattern keeps only the matching cookie, the upstream bug above../gradlew jvmTestis green, 183 tests.Note that
./gradlew detektalready fails onmainfor unrelated missing-documentation findings incdp; nothing here adds to it, and the CI does not run it.