Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New methods / extensions - Happy to write them #53

Closed
luke-hill opened this issue Dec 3, 2024 · 5 comments
Closed

New methods / extensions - Happy to write them #53

luke-hill opened this issue Dec 3, 2024 · 5 comments

Comments

@luke-hill
Copy link
Contributor

Finding I need the following a lot. I've already written some of them straw man. Asking if you want them uploading to the gem proper and would accept PR's to add them?

Cookie

CookieJar

  • #include? (Boolean checker)
  • #cookie_named (Detector)
@flavorjones
Copy link
Member

Hi @luke-hill, thanks for starting this conversation and offering to contribute!

  • Cookie#to_h:
    • This seems like a sensible feature to support, since cookies are just a bag of key/value pairs. Not sure what your current implementation does, but I think the PERSISTENT_PROPERTIES constant should probably be the set of keys in the hash, to keep it consistent with the YAML dump and load methods.
  • Cookie#expires:
    • Thank you for the PR! I reviewed it.
  • CookieJar#include?:
    • This already exists, though it may not be obvious. CookieJar includes Enumerable and so we get it for free.
  • CookieJar#cookie_named:
    • Can you say more about your use case here? Is the implementation similar to jar.cookies.find_all { |c| c.name == "foo" }?

@luke-hill
Copy link
Contributor Author

Hi there.

So #include? I have written as detecting it based on the name prop. This errors currently when checking the cookie jar.

So my method is cookie_jar.include?('foo') which looks inside the jar and checks whether (Using #any?), whether there is any cookie with the :name prop

cookie_named is the same as above, but using detect to return the first matcher

@flavorjones
Copy link
Member

So #include? I have written as detecting it based on the name prop

OK, then to avoid breaking anyone who is depending on the current behavior of CookieJar#include?, maybe we could introduce a new method named #include_name?

Or, alternatively, we could align the names of the two proposed cookie jar methods to be:

  • CookieJar#cookie_named
  • CookieJar#cookie_named?
    • which could be implemented as !cookie_named(name).nil?

WDYT?

@luke-hill
Copy link
Contributor Author

Sounds great. I'm actually not going to be looking at this for a while. Because I'm off for a few days. I'll copy and paste my monkeypatch file and stuff I've been working on so you can see what I can do / need then I'll adapt / work on things on my return unless some of it has been priorly done

# frozen_string_literal: true

module HTTP
  class Cookie
    # this is a rudimentary fix for the one we did, so ignore this
    def expires=(t)
      t = t.to_time
      @max_age = nil
      @session = t.nil?
      @expires = t
    end
    
    # this allows me to cast something into a format I can then inject into Selenium. So I have 2 way conversions working reliably (i.e. take selenium cookie, save as http cookie, and vice versa - I'm not 100% sure this is a reliable answer, so it might need adapting)
    def to_h
      {
        name: name,
        value: value,
        path: path,
        secure: secure,
        expires: expires
      }
    end
  end
end

module HTTP
  class CookieJar
    # I actually made this when I "thought" I could do a create unless it's already there action. However for my single use case so far, the cookie will NEVER be there, because you cannot create cookies X-Domain. So for now this method has no use for me personally.
    def cookie_named(name)
      cookies.detect { |cookie| cookie.name == name }
    end

    # This method was the companion to the above. For some reason the regular `#include?` doesn't work and gives a weird error relating to `[]` not being manipulable. I cannot remember explicitly if I was doing something wrong or not. Maybe I was :man_shrugging: 
    def include?(cookie_name)
      cookies.any? { |cookie| cookie.name == cookie_name }
    end
  end
end

@flavorjones
Copy link
Member

Seems like a reasonable start! I'm going to close this issue, but please do open PRs when you get time, I'd be happy to review and help get these ideas merged. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants