-
Notifications
You must be signed in to change notification settings - Fork 7
WIP: inline enums schemas #631
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR fixes an issue where the removeEnums
transformation was too aggressive, leaving empty schema objects that were still referenced elsewhere in the OpenAPI document. The fix intelligently handles enum-only schemas by inlining their type definitions where referenced and removing the original empty schemas.
- Enhanced
removeEnums
transformation to identify and handle enum-only schemas that are referenced via$ref
- Added logic to inline type definitions (preserving metadata like description, title, example) where enum schemas are referenced
- Implemented comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
tools/transformer/src/transformations/removeEnums.js | Core implementation of enum-only schema detection, inlining, and removal logic |
tools/transformer/tests/removeEnums.test.js | Comprehensive test suite covering various scenarios including oneOf references and complex schemas |
const referencedSchemas = new Set(); | ||
allRefs.forEach((ref) => { | ||
const refParts = ref.split("/"); | ||
if (refParts[1] === "components" && refParts[2] === "schemas") { |
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.
The hardcoded array indices for ref parsing (refParts[1], refParts[2], refParts[3]) could fail if the reference format is unexpected. Consider adding validation to ensure refParts has the expected length and structure before accessing these indices.
if (refParts[1] === "components" && refParts[2] === "schemas") { | |
if (refParts.length >= 4 && refParts[1] === "components" && refParts[2] === "schemas" && refParts[3]) { |
Copilot uses AI. Check for mistakes.
// Remove the schema | ||
delete api.components.schemas[schemaName]; | ||
|
||
console.info(`Removed enum-only schema '${schemaName}' and inlined its type definition`); |
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.
[nitpick] Using console.info for logging in a transformation function may not be appropriate for all environments. Consider using a proper logging framework or making logging configurable.
Copilot uses AI. Check for mistakes.
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.
Remember: That is approved logging for Transformation
if (key === "$ref" && obj[key] === targetRef) { | ||
// Replace the $ref with the inline definition | ||
delete obj[key]; | ||
Object.assign(obj, inlineDefinition); |
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.
Using Object.assign to replace the $ref object modifies the original object structure. This could potentially cause issues if other parts of the code expect the original object structure. Consider creating a new object instead of mutating the existing one.
Object.assign(obj, inlineDefinition); | |
const newObj = { ...obj, ...inlineDefinition }; | |
Object.keys(obj).forEach(k => delete obj[k]); // Clear the original object | |
Object.assign(obj, newObj); // Copy new properties into the original object |
Copilot uses AI. Check for mistakes.
Fix for Enum Removal Leaving Empty Referenced Objects
Problem
The original
removeEnums
transformation was too aggressive - it removed allenum
fields throughout the OpenAPI document, including from schemas that were primarily enum definitions and were referenced elsewhere via$ref
.This caused problems such as:
AlertAuditTypeView
that were purely enum definitions became essentially empty after enum removal$ref
in other parts of the documentExample of the Problem
Before transformation:
After old transformation:
Solution
The enhanced
removeEnums
transformation now handles this case intelligently:Identify enum-only schemas: Detect schemas that are primarily enum definitions (have
enum
field,type
field, but noproperties
,allOf
,oneOf
,anyOf
, oritems
)Check if referenced: Only process schemas that are actually referenced via
$ref
elsewhere in the documentInline and remove: For such schemas:
$ref
references with an inline type definition (preserving description, example, title, but removing enum)Continue normal processing: Remove enum fields from all other schemas as before
After new transformation:
Benefits