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

Feature/prop #3924

Draft
wants to merge 12 commits into
base: series/3.x
Choose a base branch
from
4 changes: 3 additions & 1 deletion core/shared/src/main/scala/cats/effect/IO.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import cats.data.Ior
import cats.effect.instances.spawn
import cats.effect.kernel.CancelScope
import cats.effect.kernel.GenTemporal.handleDuration
import cats.effect.std.{Backpressure, Console, Env, Supervisor, UUIDGen}
import cats.effect.std.{Backpressure, Console, Env, Prop, Supervisor, UUIDGen}
import cats.effect.tracing.{Tracing, TracingEvent}
import cats.effect.unsafe.IORuntime
import cats.syntax._
Expand Down Expand Up @@ -2108,6 +2108,8 @@ object IO extends IOCompanionPlatform with IOLowPriorityImplicits with TuplePara

implicit val envForIO: Env[IO] = Env.make

implicit val propForIO: Prop[IO] = Prop.make

// This is cached as a val to save allocations, but it uses ops from the Async
// instance which is also cached as a val, and therefore needs to appear
// later in the file
Expand Down
142 changes: 142 additions & 0 deletions std/shared/src/main/scala/cats/effect/std/Prop.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
* Copyright 2020-2024 Typelevel
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package cats.effect.std

import cats.{~>, Applicative, Functor}
import cats.data.{EitherT, IorT, Kleisli, OptionT, ReaderWriterStateT, StateT, WriterT}
import cats.effect.kernel.Sync
import cats.kernel.Monoid

import org.typelevel.scalaccompat.annotation._

import scala.collection.immutable.Map

trait Prop[F[_]] { self =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make this sealed, just for future-proofing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, this prevents mocking. Our current practice is not really to sealed anything, for better or for worse 🤷


/**
* Retrieves the value for the specified key.
*/
def get(key: String): F[Option[String]]

/**
* Sets the value for the specified key to `value` disregarding any previous value for the
* same key.
*/
def set(key: String, value: String): F[Unit]

/**
* Removes the property.
*/
def unset(key: String): F[Unit]

def entries: F[Map[String, String]]

def mapK[G[_]](f: F ~> G): Prop[G] = new Prop[G] {
def get(key: String): G[Option[String]] = f(self.get(key))
def set(key: String, value: String): G[Unit] = f(self.set(key, value))
def unset(key: String) = f(self.unset(key))
def entries: G[Map[String, String]] = f(self.entries)
}
}

object Prop {

/**
* Summoner method for `Prop` instances.
*/
def apply[F[_]](implicit ev: Prop[F]): ev.type = ev

/**
* Constructs a `Prop` instance for `F` data types that are [[cats.effect.kernel.Sync]].
*/
def make[F[_]](implicit F: Sync[F]): Prop[F] = new SyncProp[F]

/**
* [[Prop]] instance built for `cats.data.EitherT` values initialized with any `F` data type
* that also implements `Prop`.
*/
implicit def catsEitherTProp[F[_]: Prop: Functor, L]: Prop[EitherT[F, L, *]] =
Prop[F].mapK(EitherT.liftK)

/**
* [[Prop]] instance built for `cats.data.Kleisli` values initialized with any `F` data type
* that also implements `Prop`.
*/
implicit def catsKleisliProp[F[_]: Prop, R]: Prop[Kleisli[F, R, *]] =
Prop[F].mapK(Kleisli.liftK)

/**
* [[Prop]] instance built for `cats.data.OptionT` values initialized with any `F` data type
* that also implements `Prop`.
*/
implicit def catsOptionTProp[F[_]: Prop: Functor]: Prop[OptionT[F, *]] =
Prop[F].mapK(OptionT.liftK)

/**
* [[Prop]] instance built for `cats.data.StateT` values initialized with any `F` data type
* that also implements `Prop`.
*/
implicit def catsStateTProp[F[_]: Prop: Applicative, S]: Prop[StateT[F, S, *]] =
Prop[F].mapK(StateT.liftK)

/**
* [[Prop]] instance built for `cats.data.WriterT` values initialized with any `F` data type
* that also implements `Prop`.
*/
implicit def catsWriterTProp[
F[_]: Prop: Applicative,
L: Monoid
]: Prop[WriterT[F, L, *]] =
Prop[F].mapK(WriterT.liftK)

/**
* [[Prop]] instance built for `cats.data.IorT` values initialized with any `F` data type that
* also implements `Prop`.
*/
implicit def catsIorTProp[F[_]: Prop: Functor, L]: Prop[IorT[F, L, *]] =
Prop[F].mapK(IorT.liftK)

/**
* [[Prop]] instance built for `cats.data.ReaderWriterStateT` values initialized with any `F`
* data type that also implements `Prop`.
*/
implicit def catsReaderWriterStateTProp[
F[_]: Prop: Applicative,
E,
L: Monoid,
S
]: Prop[ReaderWriterStateT[F, E, L, S, *]] =
Prop[F].mapK(ReaderWriterStateT.liftK)

private[std] final class SyncProp[F[_]](implicit F: Sync[F]) extends Prop[F] {

def get(key: String): F[Option[String]] =
F.delay(Option(System.getProperty(key))) // thread-safe

def set(key: String, value: String): F[Unit] =
F.void(F.delay(System.setProperty(key, value)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading the JDK source correctly, this eventually calls a synchronized method. So, technically, it should be F.blocking. I don't know if we care about this though...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. I think we should do that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
F.void(F.delay(System.setProperty(key, value)))
F.void(F.blocking(System.setProperty(key, value)))


def unset(key: String): F[Unit] = F.void(F.delay(System.clearProperty(key)))

@nowarn213("cat=deprecation")
@nowarn3("cat=deprecation")
def entries: F[Map[String, String]] = {
import scala.collection.JavaConverters._
F.delay(Map.empty ++ System.getProperties().asScala)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I think this calls into a Collections.synchronizedSet, so technically should be blocking. (Although this should be uncontended, I think. So we probably care even less...)

}
}
}
45 changes: 45 additions & 0 deletions tests/shared/src/test/scala/cats/effect/std/PropSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright 2020-2024 Typelevel
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package cats.effect
package std

class PropSpec extends BaseSpec {

"Prop" should {
"retrieve a property just set" in real {
Random.javaUtilConcurrentThreadLocalRandom[IO].nextString(12).flatMap { key =>
Prop[IO].set(key, "bar") *> Prop[IO].get(key).flatMap(x => IO(x mustEqual Some("bar")))
}
}
"return none for a non-existent property" in real {
Prop[IO].get("MADE_THIS_UP").flatMap(x => IO(x must beNone))
}
"unset" in real {
Random.javaUtilConcurrentThreadLocalRandom[IO].nextString(12).flatMap { key =>
Prop[IO].set(key, "bar") *> Prop[IO]
.unset(key) *> Prop[IO].get(key).flatMap(x => IO(x must beNone))
}
}
"retrieve the system properties" in real {
for {
_ <- Prop[IO].set("some property", "the value")
props <- Prop[IO].entries
assertion <- IO(props mustEqual System.getProperties())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a test failure here, I think it's just because we're comparing a Scala Map with a Java hashtable.

} yield assertion
}
}
}
Loading