playframework / playframework/playframework

Have consistent names for adding, removing and clearing data in multiple APIs

Open
#8,779 13 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

good first issue help wanted type:improvement
Dominant language
Scala
Stars
12.6k
Forks
4k
Avg merge
2d 3h
Merged PRs (30d)
29

Description

Purpose

We don't have good/consistent names for APIs that manipulates headers, session, flash, and cookies. For example, we have discardCookies (it is discarding in other places), removingFromSession and clearlingLang. And in some other places just remove or -.

How to make the change

To enable a smooth migration, we need to deprecate the existing methods and add the new methods. Just renaming or removing will break binary compatibility which won't give users a change to migrate at their own pace.

Methods to rename

Here is a (not extensive) compilation:

play.api.mvc.Session {
  def get(key: String): Option[String]
  def +(kv: (String, String)): Session
  def -(key: String): Session
  def apply(key: String): String = data(key)
}

play.api.mvc.Flash {
  def get(key: String): Option[String]
  def +(kv: (String, String)): Flash
  def -(key: String): Flash
  def apply(key: String): String
}

play.api.mvc.Result {
    def withHeaders(headers: (String, String)*)
    def withDateHeaders(headers: (String, ZonedDateTime)*)
    def discardingHeader(name: String)
    def withCookies(cookies: Cookie*)
    def discardingCookies(cookies: DiscardingCookie*)
    def withSession(session: Session)
    def withSession(session: (String, String)*)
    def withNewSession
    def flashing(flash: Flash)
    def flashing(values: (String, String)*)
    def as(contentType: String)
    def session(implicit request: RequestHeader): Session = newSession getOrElse request.session
    def addingToSession(values: (String, String)*)(implicit request: RequestHeader)
    def removingFromSession(keys: String*)(implicit request: RequestHeader)
}

play.mvc.Result {
    public Optional<String> header(String header)
    public Map<String, String> headers()
    public Result withFlash(Flash flash)
    public Result withFlash(Map<String, String> flash)
    public Result withNewFlash()
    public Result flashing(Map<String, String> values)
    public Result flashing(String key, String value)
    public Result removingFromFlash(String... keys)
    public Session session()
    public Session session(Http.Request request)
    public Result withSession(Session session)
    public Result withSession(Map<String, String> session)
    public Result withNewSession()
    public Result addingToSession(Http.Request request, Map<String, String> values)
    public Result addingToSession(Http.Request request, String key, String value)
    public Result removingFromSession(Http.Request request, String... keys)
    public Cookie cookie(String name)
    public Optional<Cookie> getCookie(String name)
    public Cookies cookies()
    public Result withCookies(Cookie... newCookies) {
    public Result discardCookie(String name)
    public Result discardCookie(String name, String path)
    public Result discardCookie(String name, String path, String domain)
    public Result discardCookie(String name, String path, String domain, boolean secure)
    public Result withHeader(String name, String value)
    public Result withHeaders(String... nameValues)
    public Result discardHeader(String name)
    public Result withLang(Lang lang, MessagesApi messagesApi)
    public Result clearingLang(MessagesApi messagesApi)
}

play.api.mvc.Request {
    def queryString: Map[String, Seq[String]] = target.queryMap
    def headers: Headers
    def withHeaders(newHeaders: Headers): RequestHeader =
    def attrs: TypedMap
    def withAttrs(newAttrs: TypedMap): RequestHeader =
    def addAttr[A](key: TypedKey[A], value: A): RequestHeader =
    def removeAttr(key: TypedKey[_]): RequestHeader =
    def getQueryString(key: String): Option[String] = target.getQueryParameter(key)
    def cookies: Cookies = attrs(RequestAttrKey.Cookies).value
    def session: Session = attrs(RequestAttrKey.Session).value
    def flash: Flash = attrs(RequestAttrKey.Flash).value
    def rawQueryString: String = target.queryString
    def withTransientLang(lang: Lang): RequestHeader =
    def withTransientLang(code: String): RequestHeader =
    def withTransientLang(locale: Locale): RequestHeader =
    def clearTransientLang(): RequestHeader =
    def transientLang(): Option[Lang] =
}

play.mvc.Request {
    TypedMap attrs();
    RequestHeader withAttrs(TypedMap newAttrs);
    <A> RequestHeader addAttr(TypedKey<A> key, A value);
    RequestHeader removeAttr(TypedKey<?> key);
    Map<String,String[]> queryString();
    String getQueryString(String key);
    Cookies cookies();
    Cookie cookie(String name);
    default Session session()
    default Flash flash()
    Headers getHeaders();
    default Optional<String> header(String headerName)
    default boolean hasHeader(String headerName) {
    default RequestHeader withTransientLang(Lang lang) {
    default RequestHeader withTransientLang(String code) {
    default RequestHeader withTransientLang(Locale locale) {
    default RequestHeader clearTransientLang() {
    default Optional<Lang> transientLang() {
}

play.api.mvc.Headers {
  def hasHeader(headerName: String): Boolean = get(headerName).isDefined
  def add(headers: (String, String)*): Headers = new Headers(this.headers ++ headers)
  def apply(key: String): String = get(key).getOrElse(scala.sys.error("Header doesn't exist"))
  def get(key: String): Option[String] = getAll(key).headOption
  def getAll(key: String): Seq[String] = toMap.getOrElse(key, Nil)
  def keys: Set[String] = toMap.keySet
  def remove(keys: String*): Headers = {
  def replace(headers: (String, String)*): Headers = remove(headers.map(_._1): _*).add(headers: _*)
}

play.mvc.Http.Headers {
    public Map<String, List<String>> toMap()
    public boolean contains(String headerName)
    public Optional<String> get(String name)
    public List<String> getAll(String name)
    public Headers addHeader(String name, String value)
    public Headers addHeader(String name, List<String> values)
    public Headers remove(String name)
}

Originally posted by @marcospereira in https://github.com/playframework/playframework/pull/8768#issuecomment-436084836

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the listed play.api.mvc and play.mvc API classes, including Session, Flash, Result, Request, and Headers. Review the existing method names and compatibility requirements, then define consistent replacements with deprecations; done means the affected APIs follow the agreed naming scheme without breaking binary compatibility.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, scala
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.