-
Notifications
You must be signed in to change notification settings - Fork 203
Fix OTP 28 compatibility and test issues #668
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
Fix OTP 28 compatibility and test issues #668
Conversation
This fixes the failing test but not the root cause, as the examples, docs, and typespec still support generating a struct-based schema with a regex pattern as a macro. Lines 183 to 194 in 410f3aa
open_api_spex/lib/open_api_spex.ex Line 209 in 410f3aa
open_api_spex/lib/open_api_spex/schema.ex Line 228 in 410f3aa
For backwards compatibility See my attempt at this in PR matthewsinclair#1 |
lib/open_api_spex.ex
Outdated
|> Enum.reject(&is_nil/1) | ||
|> Enum.join("\n\n") | ||
end | ||
|
||
def schema, do: @schema | ||
if OpenApiSpex.Schema.has_regex_pattern?(unquote(body)) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this change is only necessary for OTP 28 and it might introduce a performance penalty, should it affect applications not running on OTP 28?
@@ -530,4 +530,24 @@ defmodule OpenApiSpex.Schema do | |||
defp default(value) do | |||
raise "Expected %Schema{}, schema module, or %Reference{}. Got: #{inspect(value)}" | |||
end | |||
|
|||
@doc false | |||
def has_regex_pattern?(%Schema{pattern: %Regex{}}), do: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion is to only log a warning when a Regex is used in conjunction with OTP 28, with a message to update their code to pattern: String.t
.
3f1dc20
to
b3d4f53
Compare
…ling - Implement OTP version detection in schema macro - Add runtime schema compilation for OTP 28+ when regex patterns are present - Maintain module attribute optimization for non-regex schemas on all OTP versions - Add deprecation warning for regex patterns on OTP 28+ with migration guidance - Fix cast_parameters.ex to avoid regex in module attributes - Update string test to check pattern source instead of regex struct equality This ensures backward compatibility while providing a clear migration path for OTP 28+ users to move from regex patterns to string patterns.
b3d4f53
to
2ed65d2
Compare
should this PR be closed in favour of your other one? Which one is more complete? |
Yes. Apologies for the double-up. There were two separate comment threads, and I think our changes crossed in the aether. |
Fixes multiple OTP 28 compatibility issues and test failures.
Problem
OTP 28 introduced stricter validation for module attributes that contain references, compiled regexes, or other complex data structures that cannot be properly escaped. This affects libraries that store compiled schemas or parsers in module attributes at compile time.
OTP 28 Compatibility Fixes
1. Module Attribute to Runtime Function
File:
lib/open_api_spex/cast_parameters.ex
@default_parsers
module attribute containing compiled regex patterns cannot be escaped under OTP 28default_content_parsers/0
private function2. Schema Pattern Format
File:
test/support/schemas.ex
~r/[a-zA-Z][a-zA-Z0-9_]+/
in schema definition"[a-zA-Z][a-zA-Z0-9_]+"
Test Compatibility Fixes
3. Regex Comparison in Tests
File:
test/cast/string_test.exs
regex.source
instead of regex objects4. Phoenix Router API Update
File:
test/operation_test.exs
Phoenix.Router.Route.build/13
changed tobuild/14
in Phoenix 1.7+Testing Results
Changes Summary
This resolves compilation errors on OTP 28 while maintaining identical functionality and ensuring all tests pass.