Migrate from `doobie-util` to `ashtray` project space. by arashi01 · Pull Request #64 · shuwariafrica/ashtray

arashi01 · 2025-12-06T11:37:17Z

Introduce UniqueIdentifier type for MSSQL identifiers.

Introduce `UniqueIdentifier` type for MSSQL identifiers.

Copilot

Pull request overview

This PR migrates the project from doobie-util under the africa.shuwari.doobie namespace to ashtray under the ashtray package space. The major addition is a comprehensive type-safe Identifier type for SQL Server UNIQUEIDENTIFIER handling with phantom-typed versioned wrappers, compile-time validated literals, and effectful generation.

Key changes:

Introduces Identifier opaque type with Versioned[V] phantom wrapper for type-safe UUID version handling (V1, V4, V7)
Adds compile-time validated identifier literals (id"...", idv7"...", etc.) with macro-based validation
Migrates from manual docker-compose setup to testcontainers-scala for integration tests
Updates dependencies (Scala 3.7.4, doobie 1.0.0-RC11, newer test frameworks) and CI configuration

Reviewed changes

Copilot reviewed 31 out of 32 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
build.sbt	Restructures modules to `ashtray-mssql` and `ashtray-zio-prelude`; updates dependencies and SCM URLs (contains typos)
modules/mssql/src/main/scala/ashtray/mssql/Identifier.scala	Implements core `Identifier` opaque type with versioned wrapper and comprehensive parsing/encoding logic
modules/mssql/src/main/scala/ashtray/mssql/Version.scala	Defines UUID version marker sealed trait with V1, V4, V7, and Unknown cases
modules/mssql/src/main/scala/ashtray/mssql/IdentifierGen.scala	Provides effectful identifier generation with V4 and V7 generators using SecureRandom
modules/mssql/src/main/scala/ashtray/mssql/identifier_literal.scala	Implements compile-time validated string interpolators for identifier literals
modules/mssql/src/main/scala/ashtray/mssql/IdentifierError.scala	Defines error algebra for identifier parsing and validation
modules/mssql/src/main/scala/ashtray/mssql/meta.scala	Migrates Meta instances to new Identifier types with SQL Server byte layout handling
modules/mssql/src/main/scala/ashtray/mssql/time.scala	Refines formatters to enforce SQL Server's 7-digit fractional second precision
modules/mssql/src/main/scala/ashtray/mssql/package.scala	Adds type aliases and documentation for package object
modules/test/src/main/scala/ashtray/test/test_database.scala	Replaces manual docker-compose with testcontainers-scala MSSQLContainerSuite
modules/zio-prelude/src/main/scala/ashtray/prelude/package.scala	Migrates package namespace from `africa.shuwari.doobie` to `ashtray`
modules/zio-prelude/src/test/scala/ashtray/prelude/test/newtype_tests.scala	Updates tests to use new testcontainers infrastructure
modules/mssql/src/test/scala/ashtray/mssql/test/*.scala	Adds comprehensive test coverage for identifier parsing, encoding, generation, and database round-trips
project/plugins.sbt	Updates plugin versions to latest releases
.scalafmt.conf	Adds docstring formatting rules and updates to Scala3 dialect
.scalafix.conf	Refines import grouping and enables unused import removal
.github/workflows/build.yml	Modernizes CI workflow with testcontainers support (removes manual docker-compose)
README.md	Complete rewrite documenting new ashtray modules and Identifier functionality
docker-compose.yml	Removed (replaced by testcontainers)
project/containers/mssql/*	Removed (replaced by testcontainers)

Comments suppressed due to low confidence (5)

modules/mssql/src/main/scala/ashtray/mssql/time.scala:29

The comment says "strict parsing" but the actual implementation change involves specific precision handling for fractional seconds (0-7 digits). Consider clarifying that this is specifically to enforce SQL Server's 7-digit max precision for nanoseconds, e.g., "DATETIME2 formatter enforcing SQL Server's max 7-digit fractional second precision."
modules/mssql/src/main/scala/ashtray/mssql/time.scala:38
Similar to line 29, the comment could be more specific about the fractional second precision constraint. Consider clarifying: "DATETIMEOFFSET formatter with SQL Server's max 7-digit fractional second precision and strict offset format."
modules/mssql/src/main/scala/ashtray/mssql/meta.scala:49
Throwing the error directly in a Meta instance could result in unclear exceptions during database operations. Consider wrapping the error in a more descriptive exception that indicates this occurred during UNIQUEIDENTIFIER decoding, e.g., throw new SQLException(s"Failed to decode UNIQUEIDENTIFIER: ${err.getMessage}", err).
modules/mssql/src/main/scala/ashtray/mssql/meta.scala:64
Similar to line 49, throwing the error directly in the UUID Meta instance could result in unclear exceptions. Consider wrapping with a more descriptive SQLException that indicates the context of the failure during UUID decoding from UNIQUEIDENTIFIER.
modules/mssql/src/main/scala/ashtray/mssql/meta.scala:45
The use of null.asInstanceOf[Identifier] is unsafe. Consider using Option[Identifier] in the Meta instance or documenting why null handling is required here. The scalafix:ok comment suggests this is intentional, but this could lead to null pointer exceptions downstream.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-06T11:44:01Z

modules/test/src/main/scala/ashtray/test/test_database.scala

+  */
+trait MSSQLContainerSuite extends FunSuite with TestContainerForAll:
+
+  // Use the same RHEL-based SQL Server 2025 image as in docker-compose/Dockerfile


The comment states "SQL Server 2025 image" but SQL Server 2025 has not been released (current year is December 2025). This is likely a mistake or the container image is actually a preview/CTP version. Please clarify the actual version being used or correct the comment to indicate it's a preview version if applicable.

Suggested change

// Use the same RHEL-based SQL Server 2025 image as in docker-compose/Dockerfile

// Use the same RHEL-based SQL Server 2025 *preview* (CTP) image as in docker-compose/Dockerfile.

// Note: SQL Server 2025 has not been officially released; this image is a preview/CTP version.

Copilot · 2025-12-06T11:44:01Z

modules/mssql/src/main/scala/ashtray/mssql/Identifier.scala

+    var i = 0
+    while i < 8 do
+      array(offset + i) = (value >>> (56 - i * 8)).toByte
+      i += 1 // scalafix:ok


Similar to line 425, this scalafix:ok suppression could benefit from a brief inline comment explaining the rationale for the imperative style, such as "// Imperative loop for zero-allocation performance".

Suggested change

i += 1 // scalafix:ok

i += 1 // scalafix:ok // Imperative loop for zero-allocation performance

Copilot · 2025-12-06T11:44:02Z

modules/mssql/src/main/scala/ashtray/mssql/Identifier.scala

+  /** Unsafely build from SQL Server bytes, throwing on invalid input. */
+  def fromSqlServerBytesUnsafe(bytes: Array[Byte]): Identifier = fromSqlServerBytes(bytes) match
+    case Right(id) => id
+    case Left(err) => throw err // scalafix:ok


Similar to line 238, this unsafe throw pattern should provide better error context, such as throw new IllegalArgumentException(s"Failed to parse identifier from SQL Server bytes: ${err.getMessage}", err).

Suggested change

case Left(err) => throw err // scalafix:ok

case Left(err) => throw new IllegalArgumentException(s"Failed to parse identifier from SQL Server bytes: ${err.getMessage}", err)

Copilot · 2025-12-06T11:44:02Z

modules/mssql/src/main/scala/ashtray/mssql/Identifier.scala

+  /** Parse unsafely, throwing on invalid input. */
+  def parseUnsafe(value: String): Identifier = parse(value) match
+    case Right(id) => id
+    case Left(err) => throw err // scalafix:ok


This unsafe throw pattern should provide better error context for debugging, such as throw new IllegalArgumentException(s"Failed to parse identifier string: ${err.getMessage}", err).

Copilot · 2025-12-06T11:44:02Z

modules/mssql/src/main/scala/ashtray/mssql/identifier_literal.scala

+    var acc = 0L
+    var i = start
+    while i < end do
+      val c = s.charAt(i)
+      if skipHyphens && c == '-' then ()
+      else
+        val n = hexToNibble(c)
+        acc = (acc << 4) | n
+      i += 1
+    acc // scalafix:ok


The acc return value with a scalafix:ok comment is used to suppress warnings about returning mutable state. Consider restructuring the function to use a tail-recursive approach or a fold operation that makes the accumulator pattern more explicit and avoids the need for suppression.

Suggested change

var acc = 0L

var i = start

while i < end do

val c = s.charAt(i)

if skipHyphens && c == '-' then ()

else

val n = hexToNibble(c)

acc = (acc << 4) | n

i += 1

acc // scalafix:ok

@annotation.tailrec

def loop(i: Int, acc: Long): Long =

if i >= end then acc

else

val c = s.charAt(i)

if skipHyphens && c == '-' then loop(i + 1, acc)

else

val n = hexToNibble(c)

loop(i + 1, (acc << 4) | n)

loop(start, 0L)

Copilot · 2025-12-06T11:44:04Z

modules/mssql/src/main/scala/ashtray/mssql/Identifier.scala

+    given [V <: Version]: Order[Versioned[V]] = Identifier.given_Order_Identifier.asInstanceOf[Order[Versioned[V]]]
+    given [V <: Version]: Hash[Versioned[V]] = Identifier.given_Hash_Identifier.asInstanceOf[Hash[Versioned[V]]]
+    given [V <: Version]: Eq[Versioned[V]] = Identifier.given_Eq_Identifier.asInstanceOf[Eq[Versioned[V]]]


Using asInstanceOf to cast type class instances is unsafe and bypasses type safety. While this works because Versioned[V] is an opaque type alias over Identifier, this pattern is fragile and could break if the representation changes. Consider using proper derivation or defining the instances directly for Versioned[V] without casting.

Suggested change

given [V <: Version]: Order[Versioned[V]] = Identifier.given_Order_Identifier.asInstanceOf[Order[Versioned[V]]]

given [V <: Version]: Hash[Versioned[V]] = Identifier.given_Hash_Identifier.asInstanceOf[Hash[Versioned[V]]]

given [V <: Version]: Eq[Versioned[V]] = Identifier.given_Eq_Identifier.asInstanceOf[Eq[Versioned[V]]]

given [V <: Version]: Order[Versioned[V]] = Order.by(_.untyped)

given [V <: Version]: Hash[Versioned[V]] = Hash.by(_.untyped)

given [V <: Version]: Eq[Versioned[V]] = Eq.by(_.untyped)

Copilot · 2025-12-06T11:44:05Z

modules/mssql/src/main/scala/ashtray/mssql/identifier_literal.scala

+              (c >= 'A' && c <= 'F')
+          if !ok then break(Left(s"Invalid character '$c' at position $i in identifier literal"))
+          i += 1
+        Right(()) // scalafix:ok


The Right(()) return value with a scalafix:ok comment suggests intentional use of unit return. However, this could be simplified by using a custom sentinel value or redesigning the validation to return Option[IdentifierError] instead of Either[String, Unit], making the intent clearer and avoiding the need for the scalafix suppression.

Copilot · 2025-12-06T11:44:05Z

README.md

+
+---
+
+## Licence


Inconsistent spelling: "Licence" should be "License" to match the American English spelling used throughout the codebase (e.g., in the Apache License headers and references).

Suggested change

## Licence

## License

Copilot · 2025-12-06T11:44:05Z

modules/mssql/src/main/scala/ashtray/mssql/Identifier.scala

+  /** Unsafely build from bytes, throwing on invalid input. */
+  def fromBytesUnsafe(bytes: Array[Byte]): Identifier = fromBytes(bytes) match
+    case Right(id) => id
+    case Left(err) => throw err // scalafix:ok


The unsafe throw pattern case Left(err) => throw err is used without wrapping in a more descriptive exception. Consider providing better error context for debugging, such as throw new IllegalArgumentException(s"Failed to parse identifier from bytes: ${err.getMessage}", err).

Suggested change

case Left(err) => throw err // scalafix:ok

case Left(err) => throw new IllegalArgumentException(s"Failed to parse identifier from bytes: ${err.getMessage}", err) // scalafix:ok

Copilot · 2025-12-06T11:44:05Z

.scalafmt.conf

 rewrite.scala3.removeOptionalBraces = true

+#docstrings.style = SpaceAsterisk # Retained for manual formatting use
+docstrings.style = keep # Prevents header mangling. Allows CI checks to pass. Use `SpaceAsterisk` otherwise


The comment "Allows CI checks to pass" on line 27 suggests a workaround for CI issues. Consider documenting what specific CI issue this addresses or creating a tracking issue to eventually use the preferred SpaceAsterisk style consistently once the underlying issue is resolved.

Suggested change

docstrings.style = keep # Prevents header mangling. Allows CI checks to pass. Use `SpaceAsterisk` otherwise

docstrings.style = keep # Workaround for CI header mangling issue (see: https://github.com/your-org/your-repo/issues/1234). Allows CI checks to pass. Use `SpaceAsterisk` once resolved.

Migrate from doobie-util to ashtray project space.

6d4e120

Introduce `UniqueIdentifier` type for MSSQL identifiers.

arashi01 requested a review from Copilot December 6, 2025 11:37

Copilot started reviewing on behalf of arashi01 December 6, 2025 11:38 View session

arashi01 merged commit 763e0e7 into shuwariafrica:main Dec 6, 2025
9 checks passed

arashi01 deleted the project-migration branch December 6, 2025 11:43

Copilot AI reviewed Dec 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from `doobie-util` to `ashtray` project space.#64

Migrate from `doobie-util` to `ashtray` project space.#64
arashi01 merged 1 commit intoshuwariafrica:mainfrom
arashi01:project-migration

arashi01 commented Dec 6, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Copilot AI Dec 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// Use the same RHEL-based SQL Server 2025 image as in docker-compose/Dockerfile
	// Use the same RHEL-based SQL Server 2025 preview (CTP) image as in docker-compose/Dockerfile.
	// Note: SQL Server 2025 has not been officially released; this image is a preview/CTP version.

	i += 1 // scalafix:ok
	i += 1 // scalafix:ok // Imperative loop for zero-allocation performance

	case Left(err) => throw err // scalafix:ok
	case Left(err) => throw new IllegalArgumentException(s"Failed to parse identifier from SQL Server bytes: ${err.getMessage}", err)

	docstrings.style = keep # Prevents header mangling. Allows CI checks to pass. Use `SpaceAsterisk` otherwise
	docstrings.style = keep # Workaround for CI header mangling issue (see: https://github.com/your-org/your-repo/issues/1234). Allows CI checks to pass. Use `SpaceAsterisk` once resolved.


		---

		## Licence

Conversation

arashi01 commented Dec 6, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants