metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.44k stars 205 forks source link

Fix inconsistencies on UUID transform helper #869

Closed niwinz closed 1 year ago

niwinz commented 1 year ago

This commit fixes the https://github.com/metosin/malli/issues/867 issue. It normalizes all to an uuid regex check and then a simple platform dependent UUID object creation.

Additional:

I used to use a rebel repl + tools.namespaces + user ns for experimenting and running tests and tried to add this workflow locally (don't pretend this merged on malli. But I found that I'm unable to start repl because I constantly get exceptions on refreshing the namespaces.

Example:

~/tmp/malli niwinz-uuid-schema ⇡1 !1 ?1 ❯ ./bin/repl                                                                                                                                                                                              3s 11:09:37
+ clojure -M:shadow:rebel:test -m rebel-readline.main
[Rebel readline] Type :repl/help for online help info
user=> (r/refresh)
:reloading (malli.sci malli.registry malli.impl.util malli.impl.regex malli.core malli.generator malli.instrument malli.destructure malli.experimental malli.clj-kondo malli.util malli.error malli.dev.virhe malli.dev.pretty malli.dev malli.demo malli.provider malli.experimental.time malli.experimental.time.generator malli.experimental.time-test malli.experimental.time.generator-test malli.dot malli.plantuml malli.plantuml-test malli.dev.cljs-kondo-preload malli.transform malli.provider-test malli.json-schema malli.dot-test malli.edn malli.core-test malli.swagger malli.swagger-test malli.clj-kondo-test malli.dev-test malli.registry-test malli.app2 malli.experimental.always-test malli.dev.cljs malli.experimental.time.json-schema clojure.alpha.spec.protocols clojure.alpha.spec.gen clojure.alpha.spec clojure.alpha.spec.impl malli.experimental.describe malli.experimental-test malli.experimental.time.json-schema-test malli.error-test malli.json-schema-test malli.generator-test malli.experimental.describe-test malli.experimental.time.transform malli.instrument-test malli.util-test malli.destructure-test malli.transform-test bb-test-runner malli.experimental.lite malli.experimental.lite-test malli.dev.pretty-test malli.app malli.generator-ast malli.generator-ast-test malli.dev.virhe-test malli.experimental.time.transform-test clojure.alpha.spec.test malli.instrument.cljs user malli.generator-debug demo malli.dev.cljs-noop)
:error-while-loading malli.experimental.always-test
#error {
 :cause ":malli.core/invalid-schema"
 :data {:type :malli.core/invalid-schema, :message :malli.core/invalid-schema, :data {:schema :=>}}
 :via
 [{:type clojure.lang.Compiler$CompilerException
   :message "Syntax error macroexpanding at (malli/experimental/always_test.cljc:7:1)."
   :data #:clojure.error{:phase :execution, :line 7, :column 1, :source "malli/experimental/always_test.cljc"}
   :at [clojure.lang.Compiler load "Compiler.java" 7665]}
  {:type clojure.lang.ExceptionInfo
   :message ":malli.core/invalid-schema"
   :data {:type :malli.core/invalid-schema, :message :malli.core/invalid-schema, :data {:schema :=>}}
   :at [malli.core$_exception invokeStatic "core.cljc" 138]}]
 :trace
 [[malli.core$_exception invokeStatic "core.cljc" 138]
  [malli.core$_exception invoke "core.cljc" 136]
  [malli.core$_fail_BANG_ invokeStatic "core.cljc" 142]
  [malli.core$_fail_BANG_ invoke "core.cljc" 140]
  [malli.core$_lookup_BANG_ invokeStatic "core.cljc" 271]
  [malli.core$_lookup_BANG_ invoke "core.cljc" 267]
  [malli.core$schema invokeStatic "core.cljc" 2019]
  [malli.core$schema invoke "core.cljc" 2005]
  [malli.core$_instrument invokeStatic "core.cljc" 2522]
  [malli.core$_instrument invoke "core.cljc" 2506]
  [malli.core$_instrument invokeStatic "core.cljc" 2520]
  [malli.core$_instrument invoke "core.cljc" 2506]
  [malli.experimental.always_test$eval38365 invokeStatic "always_test.cljc" 7]
  [malli.experimental.always_test$eval38365 invoke "always_test.cljc" 7]
  [clojure.lang.Compiler eval "Compiler.java" 7194]
  [clojure.lang.Compiler load "Compiler.java" 7653]
  [clojure.lang.RT loadResourceScript "RT.java" 381]
  [clojure.lang.RT loadResourceScript "RT.java" 372]
  [clojure.lang.RT load "RT.java" 459]
  [clojure.lang.RT load "RT.java" 424]
  [clojure.core$load$fn__6908 invoke "core.clj" 6161]
[...]

I'm pretty sure that I missed something but I don't understand exactly the cause of the exception. I added all the necessary code to an additional commit that I will delete before merge, but can you please help me understand what is happening?

I don't add nothing special, just tools.namespace and call (clojure.tools.namespace.repl/refresh), the same happens if I start plain clojure repl with clj -M:shadow:rebel:test -r

ikitommi commented 1 year ago

For the refresh - it loads also malli.app2 which is meant to be a standalone app (swapping the default registry), not a test. If you exclude the app folder from refresh, it should work. The repl helpers most welcome!

ikitommi commented 1 year ago

see https://github.com/niwinz/malli/compare/niwinz-uuid-schema...metosin:malli:niwinz-uuid-schema

to watch tests from command-line, there is

./bin/kaocha --watch
niwinz commented 1 year ago

Regression tests added. Thanks for the hint of the malli.app2, now the issue is fixed.

I'm aware of kaocha --watch but I prefer just start a cli REPL and run the rests executing the function that reloads and runs the concrete test case or a namespace when I want, not when the file changes.

Then, I have added the dev/user.clj and bin/repl on the second commit, feel free to do whatever you want, merge it or discard (just tell me if you don't want the commit and I will remove it from the PR)

ikitommi commented 1 year ago

like the bin/repl and thanks for the UUID fix!

niwinz commented 1 year ago

I will check this and make a new pr for fixing that.

El dom., 26 mar. 2023 0:24, 胡雨軒 Петр @.***> escribió:

@.**** commented on this pull request.

In src/malli/transform.cljc https://github.com/metosin/malli/pull/869#discussion_r1148448120:

@@ -89,15 +89,15 @@ :else x) x))

+(def ^:private uuid-re

  • "^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$")

@.*** https://github.com/ikitommi kind poke 👋, not sure this comment is visible as this PR is merged)

— Reply to this email directly, view it on GitHub https://github.com/metosin/malli/pull/869#discussion_r1148448120, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGN7KIRXAHK6DFAHWYYVNDW555CTANCNFSM6AAAAAAV2HL24Q . You are receiving this because you authored the thread.Message ID: @.***>