refactor: 응답의 optional 필드 표기를 nullable로 통일 - #147
Conversation
📝 WalkthroughWalkthrough응답 DTO의 null 필드를 키와 함께 반환하도록 직렬화 규칙을 변경했습니다. OpenAPI에서 nullable 필드를 required로 선언하고 Changes응답 nullable 계약 표준화
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant APIClient
participant ResponseDTO
participant Jackson
participant OpenAPI
participant ContractTest
APIClient->>ResponseDTO: API 응답 요청
ResponseDTO->>Jackson: nullable 필드 포함 응답 생성
Jackson-->>APIClient: 키를 유지한 `null` 반환
ResponseDTO->>OpenAPI: required 및 nullable 스키마 제공
OpenAPI-->>ContractTest: 래퍼와 DTO 스키마 생성
ContractTest->>OpenAPI: required, nullable, `$ref` 오염 방지 검증
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #147 +/- ##
============================================
- Coverage 88.56% 88.36% -0.21%
- Complexity 506 518 +12
============================================
Files 112 112
Lines 1583 1607 +24
Branches 120 124 +4
============================================
+ Hits 1402 1420 +18
- Misses 135 138 +3
- Partials 46 49 +3
... and 4 files with indirect coverage changes
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java`:
- Around line 18-20: Update ProductResponse.from to normalize
product.getSupportedObjectives() to an empty List.of() when null, and remove
nullable = true from the supportedObjectives `@Schema` declaration so the
documented response type matches the normalized non-null list contract.
In `@src/main/java/chaeso/zip/server/common/config/NullableSchemaConfig.java`:
- Around line 26-29: Update NullableSchemaConfig to represent nullable $ref
properties with anyOf containing the referenced schema and a null ObjectSchema,
removing the ineffective allOf/nullable combination while preserving the
description. Apply the related DTO changes in GoogleAuthResponse and
SimulationItemResponse, and update OpenApiContractTest’s prefill contract
assertions to expect the new anyOf nullable schema.
In `@src/main/java/chaeso/zip/server/common/response/ApiResponse.java`:
- Around line 27-35: Update the OpenAPI examples for ApiResponse.success(...) to
explicitly include error and code as null, and for ApiResponse.fail(...) to
include data and code as null. Also update the 401 response example configured
by CommonResponsesCustomizer, which uses ApiResponse.fail(error), to include
data and code as null while preserving the existing error example.
In
`@src/test/java/chaeso/zip/server/channel/presentation/ChannelControllerTest.java`:
- Around line 225-226: Update the assertion for $.data.products[0].ctr in
ChannelControllerTest to use value(nullValue()) instead of doesNotExist(), so
the test verifies that the nullable key is present with a null value. Keep the
existing assertions for logoUrl and valueMax unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ef8e62e-eb99-4c8a-8a31-1e1e6e0610ee
📒 Files selected for processing (21)
docs/openapi.mdsrc/main/java/chaeso/zip/server/auth/application/dto/GoogleAuthResponse.javasrc/main/java/chaeso/zip/server/auth/presentation/AuthApiDocs.javasrc/main/java/chaeso/zip/server/channel/application/dto/AudienceMetricResponse.javasrc/main/java/chaeso/zip/server/channel/application/dto/ChannelDetailResponse.javasrc/main/java/chaeso/zip/server/channel/application/dto/ChannelListItemResponse.javasrc/main/java/chaeso/zip/server/channel/application/dto/PricingResponse.javasrc/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.javasrc/main/java/chaeso/zip/server/common/config/NullableSchemaConfig.javasrc/main/java/chaeso/zip/server/common/config/ResponseWrapperSchemaCustomizer.javasrc/main/java/chaeso/zip/server/common/response/ApiResponse.javasrc/main/java/chaeso/zip/server/onboarding/presentation/dto/AdHistoryRequest.javasrc/main/java/chaeso/zip/server/recommendation/application/dto/RecommendationItemResponse.javasrc/main/java/chaeso/zip/server/recommendation/presentation/RecommendationApiDocs.javasrc/main/java/chaeso/zip/server/sample/application/dto/SampleResponse.javasrc/main/java/chaeso/zip/server/simulation/application/dto/SimulationItemResponse.javasrc/main/java/chaeso/zip/server/simulation/application/dto/SimulationResponse.javasrc/main/java/chaeso/zip/server/simulation/presentation/SimulationApiDocs.javasrc/test/java/chaeso/zip/server/channel/presentation/ChannelControllerTest.javasrc/test/java/chaeso/zip/server/docs/OpenApiContractTest.javasrc/test/java/chaeso/zip/server/recommendation/presentation/RecommendationControllerTest.java
| @Schema(description = "지원 광고 목표 코드값 목록", example = "[\"AWARENESS\", \"TRAFFIC\"]", | ||
| requiredMode = Schema.RequiredMode.NOT_REQUIRED) | ||
| requiredMode = Schema.RequiredMode.REQUIRED, nullable = true) | ||
| List<CampaignObjective> supportedObjectives, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 '\bsupportedObjectives\b|getSupportedObjectives' src/main/java src/testRepository: YAPP-Github/chaeso-zip-server
Length of output: 7531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ProductResponse.java ---'
sed -n '1,80p' src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java
printf '%s\n' '--- ChannelProduct.java relevant fields and methods ---'
sed -n '1,120p' src/main/java/chaeso/zip/server/channel/domain/entity/ChannelProduct.java
printf '%s\n' '--- Migrations / SQL for supported_objectives ---'
rg -n -C 3 'supported_objectives|ChannelProduct|channel_products|create.*Product|ALTER TABLE' src/main/resources src/main/java src/test/resources src/test/java || true
printf '%s\n' '--- Tests mentioning response null support / supportedObjectives ---'
rg -n -C 3 'supportedObjectives|ProductResponse|null|지원 광고|광고 목표|requiredMode|nullable' src/test src/main/resources src/main/java/chaeso/zip/server || trueRepository: YAPP-Github/chaeso-zip-server
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- targeted ProductResponse.java ---'
node -e "const fs='fs'; const p='src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java'; const s=fs.readFileSync(p,'utf8'); const m=s.match(/@Schema\\(description = \"지원 광고 목표 코드값 목록\"[\\s\\S]*?List<ChallengeObjective> supportedObjectives,|`@Schema`\\(description = \"지원 광고 목표 코드값 목록\"[\\s\\S]*?List<.*CampaignObjective> supportedObjectives,/g);"
rg -n -C 3 '`@Schema`\(description = "지원 광고 목표 코드값 목록"|List<.*CampaignObjective> supportedObjectives|product.getSupportedObjectives\(\)' src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java src/main/java/chaeso/zip/server/channel domain entity ChannelProduct.java || true
printf '%s\n' '--- ChannelProduct exact relevant sections ---'
rg -n -C 5 'supportedObjectives|getSupportedObjectives|ChannelProduct\\(' src/main/java/chaeso/zip/server/channel/domain/entity/ChannelProduct.java
printf '%s\n' '--- data/resource references only ---'
fd -a '(\.sql|\.xml|schema|migration|migration\.)' src/main/resources src/test/resources src/main/java src/test/java 2>/dev/null | sed 's#^\./##' | head -200
rg -n -C 3 'supported_objectives|ChannelProduct|channel_products|CREATE TABLE.*Product|ALTER TABLE' src/main/resources src/test/resources src/main/java src/test/java || true
printf '%s\n' '--- focused tests around ProductResponse null behavior ---'
rg -n -C 3 'ProductResponse|supportedObjectives|jsonPath.*products.*supportedObjectives|nullValue|isEmpty\\(\\)' src/test/java src/main/java || trueRepository: YAPP-Github/chaeso-zip-server
Length of output: 1124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ProductResponse.java exact lines ---'
sed -n '1,55p' src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java
printf '%s\n' '--- ChannelProduct.java entity section ---'
sed -n '1,80p' src/main/java/chaeso/zip/server/channel/domain/entity/ChannelProduct.java
printf '%s\n' '--- supported_objectives references ---'
rg -n -C 4 'supported_objectives|supportedObjectives|supportedObjectives' src/main/resources src/test/resources src/main/java src/test/java || true
printf '%s\n' '--- ProductResponse factory mapping lines ---'
rg -n -C 3 'ProductResponse\\s*\\(|product\\.(getSupportedObjectives|getId|getProductName|getInventoryType|getMinBudgetWon|getMaxBudgetWon|get|getExpected)' src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java || trueRepository: YAPP-Github/chaeso-zip-server
Length of output: 10008
빈 목록 규약을 supportedObjectives에도 적용하세요.
supported_objectives를 nullable = true로 스웨거 스키마에 노출하면 null 응답을 허용하는 타입이 됩니다. ProductResponse.from에서는 product.getSupportedObjectives()를 그대로 전달하므로, 도메인에서 빈 목록 보장 없으면 null이[]가 아니라 null JSON으로 나옵니다. 응답 작성 시 List.of()로 정규화하고 스키마도 nullable = true를 제거하면 계약과 타입이 일치합니다.
수정 예시
`@Schema`(description = "지원 광고 목표 코드값 목록", example = "[\"AWARENESS\", \"TRAFFIC\"]",
- requiredMode = Schema.RequiredMode.REQUIRED, nullable = true)
+ requiredMode = Schema.RequiredMode.REQUIRED)
List<CampaignObjective> supportedObjectives,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/chaeso/zip/server/channel/application/dto/ProductResponse.java`
around lines 18 - 20, Update ProductResponse.from to normalize
product.getSupportedObjectives() to an empty List.of() when null, and remove
nullable = true from the supportedObjectives `@Schema` declaration so the
documented response type matches the normalized non-null list contract.
| return new ComposedSchema() | ||
| .addAllOfItem(new Schema<>().$ref(property.get$ref())) | ||
| .description(description) | ||
| .nullable(true); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(NullableSchemaConfig\.java|GoogleAuthResponse\.java|SimulationItemResponse\.java|OpenApiContractTest\.java|pom\.xml|build\.gradle|gradle\.properties)$' || true
echo "== NullableSchemaConfig =="
cat -n src/main/java/chaeso/zip/server/common/config/NullableSchemaConfig.java 2>/dev/null || true
echo "== DTO snippets =="
for f in src/main/java/chaeso/zip/server/auth/application/dto/GoogleAuthResponse.java src/main/java/chaeso/zip/server/simulation/application/dto/SimulationItemResponse.java; do
echo "--- $f"
cat -n "$f" 2>/dev/null | sed -n '1,120p'
done
echo "== contract test snippet =="
cat -n src/test/java/chaeso/zip/server/docs/OpenApiContractTest.java 2>/dev/null | sed -n '330,380p'
echo "== relevant dependency versions =="
for f in pom.xml build.gradle gradle.properties; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n "springdoc|openapi|spring-boot|jaxb|swagger" "$f" || true
fi
done
echo "== schema customizers / annotations around ref nullable =="
rg -n "`@Schema`\\(|springdoc|OASConfig|ApplicationReady|ModelConverter|ModelConverterContext|TypeContext|Customizable|SchemaConverter|OpenAPI|nullable|allOf" src/test/java src/main/java -S || trueRepository: YAPP-Github/chaeso-zip-server
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect generated/openapi files if present =="
git ls-files | rg '(openapi|swagger|api-json|api\.yaml|api\.yml|schema)' || true
echo "== behavioral/parsing probe from source snippets =="
python3 - <<'PY'
import json, textwrap, sys
paths = [
"src/main/java/chaeso/zip/server/common/config/NullableSchemaConfig.java",
"src/main/java/chaeso/zip/server/auth/application/dto/GoogleAuthResponse.java",
"src/main/java/chaeso/zip/server/simulation/application/dto/SimulationItemResponse.java",
"src/test/java/chaeso/zip/server/docs/OpenApiContractTest.java",
]
for p in paths:
print(f"--- {p}")
with open(p) as f:
lines=f.readlines()
for i,l in enumerate(lines,1):
if "ComposedSchema" in l or "new Schema<>().$ref" in l or "nullable(" in l or "assertThat(prefill.path" in l or "requiredMode = Schema.RequiredMode.REQUIRED, nullable = true" in l:
print(f"{i}: {l.rstrip()}")
PYRepository: YAPP-Github/chaeso-zip-server
Length of output: 2476
🌐 Web query:
OpenAPI 3.0.4 specification nullable allOf $ref shared schema object type nullable
💡 Result:
In OpenAPI 3.0.4, placing the nullable attribute directly alongside a $ref (at the same level) is invalid and typically ignored by tools [1][2]. This occurs because, per the specification, sibling properties on a $ref are ignored except for summary and description [2]. To make a referenced schema nullable, you should use the allOf keyword to combine the $ref with a local Schema Object that defines the nullability [1][2]. Recommended implementation: allOf: - nullable: true - $ref: '#/components/schemas/YourSchema' Alternatively, some developers use anyOf to achieve similar results, which can be more explicit in how it communicates the 'null or schema' intent [3][4]: anyOf: - nullable: true - $ref: '#/components/schemas/YourSchema' Key points regarding nullable in OpenAPI 3.0: - The nullable attribute is a modifier that, when set to true, adds null to the allowed types of the schema [5][6]. - It is only effective when applied to a Schema Object that also explicitly defines a type (e.g., type: string, type: object) [5][6]. - Because OpenAPI 3.0 schemas follow constraint-based logic, nullable cannot be inherited through $ref or allOf in a way that overrides existing, stricter type constraints in the referenced schema [7][5]. The combination must be valid independently [7]. - OpenAPI 3.0 does not support defining type: [string, null] directly as seen in later JSON Schema versions; the nullable: true modifier is the required mechanism [5][6].
Citations:
- 1: https://zenn.dev/snaka/articles/aab1cb03b9fcb4?locale=en
- 2: Normalize invalid nullable
$refschemas orval-labs/orval#3714 - 3: Clarify whether inline and reference attributes are mutually exclusive. Usage of nullable and $ref OAI/OpenAPI-Specification#2120
- 4: nullable vs type arrays (JSON Schema compatibility) OAI/OpenAPI-Specification#1389
- 5: https://github.com/OAI/OpenAPI-Specification/blob/main/proposals/2019-10-31-Clarify-Nullable.md
- 6: https://swagger.io/docs/specification/v3_0/data-models/data-types/
- 7: Reference objects don't combine well with “nullable” OAI/OpenAPI-Specification#1368
🌐 Web query:
OpenAPI 3.0.4 allOf each schema independent validation $ref schema object
💡 Result:
In OpenAPI 3.0.4, the allOf keyword functions as a composition tool based on JSON Schema principles, where each subschema defined within the array is validated independently [1][2][3]. When an instance is validated against a schema containing allOf, it must satisfy every individual subschema provided in the array for the validation to succeed [2][4][5]. Key characteristics of this behavior include: 1. Independent Validation: There is no merging or inheritance of schemas [3][5]. The validator treats each entry in the allOf array as a separate constraint that the data instance must independently pass [1][2]. 2. Schema Composition and $ref: The allOf construct is frequently used to compose models by combining a $ref to a base schema with additional inline properties or constraints [2][6]. For example, you can use allOf to extend a referenced schema by adding new fields, descriptions, or validation requirements [7][4]. 3. No Interaction Among Subschemas: Because each subschema is evaluated independently, there is no inherent interaction between the constraints defined in different subschemas [1][8]. For instance, if one subschema defines a property as a string and another subschema attempts to define the same property as an integer, the resulting schema will be logically invalid because a single value cannot satisfy both constraints simultaneously [3][6]. 4. Not Inheritance: While allOf is often referred to as "inheritance" or "composition," it does not imply a hierarchy [2][5]. If you require polymorphic behavior where an instance should match one of several possible schemas, you should use oneOf or anyOf, typically paired with a discriminator to explicitly define the relationship [9][2][6]. 5. Logical Consistency: Since validation is independent, you must ensure that your combined subschemas do not create contradictory constraints [3][6]. All provided subschemas must be simultaneously satisfiable for the data instance [2].
Citations:
- 1: Clarification regarding the
allOfcomposition when the given objects have a same property field OAI/OpenAPI-Specification#3428 - 2: https://swagger.io/docs/specification/v3_0/data-models/oneof-anyof-allof-not/
- 3: Different types of properties behavior in schemas with `allOf` OAI/OpenAPI-Specification#3103
- 4: Consider how to handle valid
allOfconstraints - schema composition hashicorp/terraform-plugin-codegen-openapi#56 - 5: https://stackoverflow.com/questions/79823407/best-practice-use-of-allof-in-openapi-specification
- 6: https://swagger.io/docs/specification/v3_0/data-models/inheritance-and-polymorphism/
- 7: In schema objects, using allOf to add a description to a $ref? OAI/OpenAPI-Specification#2033
- 8: Question: is putting
requiredby itself in anallOfblock valid jsonschema? OAI/OpenAPI-Specification#1608 - 9: Understanding references to polymorphic types with "allOf" and "discriminator" OAI/OpenAPI-Specification#3951
🌐 Web query:
OpenAPI 3.0 specification $ref with siblings allOf each schema validated against allOf member
💡 Result:
In OpenAPI 3.0, the $ref field behaves according to the JSON Reference specification, which dictates that any sibling elements appearing alongside $ref at the same level must be ignored [1][2][3]. Consequently, if you attempt to add properties like description, type, or example as direct siblings to a $ref, those properties will have no effect [1][2]. To work around this limitation in OpenAPI 3.0, the recommended approach is to use allOf [4][5]. By wrapping the $ref within an allOf array, you can effectively combine the referenced schema with additional properties [6][7]. Example usage: yaml components: schemas: MySchema: type: string ExtendedSchema: allOf: - $ref: "`#/components/schemas/MySchema`" description: "An extended description that will be respected." example: "example value" When using this allOf pattern: 1. The referenced schema (e.g., MySchema) is evaluated [7]. 2. The sibling properties defined alongside the allOf (such as description or example) are applied [4][5][7]. 3. This pattern is primarily useful for adding informational properties like description or summary [7]. It is not a mechanism for overriding existing properties within the referenced schema; attempts to override validation-impacting properties (like type) via allOf often lead to undefined or invalid behavior, depending on the tool implementation [8][7]. This behavior is specific to OpenAPI 3.0. OpenAPI 3.1 introduced compatibility with newer JSON Schema drafts, which allow for sibling properties alongside $ref, making the allOf workaround unnecessary for many use cases in later versions [8][5][7].
Citations:
- 1: https://github.com/swagger-api/swagger.io/blob/stage/src/content/docs/specification/v3_0/using-ref.md
- 2: https://swagger.io/docs/specification/v3_0/using-ref/
- 3: Allow reference siblings swagger-api/swagger-ui#4732
- 4: In schema objects, using allOf to add a description to a $ref? OAI/OpenAPI-Specification#2033
- 5: https://stackoverflow.com/questions/33629750/swagger-schema-properties-ignored-when-using-ref-why
- 6: https://redocly.com/learn/openapi/ref-guide
- 7: https://redocly.com/learn/openapi/all-of
- 8: Feature Request: Allow sibling elements with $ref that overrides the references definition OAI/OpenAPI-Specification#2744
🌐 Web query:
OpenAPI 3.0 specification "any applicable schemas" allOf each Schema Object must validate instance independently
💡 Result:
Yes, in OpenAPI 3.0 (and the underlying JSON Schema specification it relies on), the allOf keyword requires that the instance being validated must successfully validate against each of the schema objects defined within the allOf array independently [1][2][3][4]. Key technical points regarding this behavior include: 1. Independent Validation: Each subschema inside the allOf array is evaluated separately against the instance [1][5][4]. An instance is only considered valid if it satisfies every single subschema provided [2][3][6]. 2. Composition, Not Merging: Although allOf is frequently used to compose models (e.g., combining a base model with specific additions), it does not perform a structural merge of the schemas themselves [7][5]. Because validation is independent, constraints are applied cumulatively [1][5]. This can lead to unexpected results if multiple subschemas define conflicting constraints, such as different types or conflicting additionalProperties settings [1][7][4]. 3. Cumulative Constraints: Because all schemas must be satisfied, the intersection of all constraints determines the final valid state [1]. For example, if one subschema requires property A and another subschema restricts additional properties, the resulting object must satisfy both: it must have property A and it cannot have properties not explicitly defined in the relevant subschemas [1][4]. 4. Specification Source: OpenAPI 3.0 follows JSON Schema validation logic for this behavior [8][9]. The JSON Schema specification explicitly states that an instance validates against allOf if it validates successfully against all schemas defined by the keyword [3]. Because of this independent validation, developers should be cautious when using allOf to override properties, as the resulting schema must still be compatible with all previous subschemas [7][5][4].
Citations:
- 1: Details of model composition should be documented OAI/OpenAPI-Specification#1428
- 2: https://swagger.io/docs/specification/v3%5F0/data-models/oneof-anyof-allof-not/
- 3: https://github.com/json-schema-org/json-schema-spec/blob/main/specs/jsonschema-core.md
- 4: allOff with required returning Record<string, never> openapi-ts/openapi-typescript#1474
- 5: https://stackoverflow.com/questions/79823407/best-practice-use-of-allof-in-openapi-specification
- 6: https://swagger.io/docs/specification/v3_0/data-models/oneof-anyof-allof-not/
- 7: Overriding object keys in allOf - valid or undefined behaviour? OAI/OpenAPI-Specification#3069
- 8: https://swagger.io/specification/v3/
- 9: https://spec.openapis.org/oas/v3.0
🌐 Web query:
OpenAPI 3.0 $ref sibling schema properties anyOf type object $ref nullable
💡 Result:
In OpenAPI 3.0, the Reference Object (defined by the $ref keyword) does not allow sibling properties [1][2][3]. According to the specification, any sibling properties accompanying a $ref are ignored by compliant tools [1][4][5]. To include additional properties—such as nullable, examples, or descriptions—alongside a reference, you must use composition keywords like allOf or anyOf [4][6][7]. For example, to correctly define a nullable reference in OpenAPI 3.0, you should wrap the reference in an anyOf or allOf block [8][6][7]: properties: myProperty: anyOf: - $ref: '#/components/schemas/MyObject' - type: 'null' nullable: true In this structure, the nullable keyword applies to the schema object itself, while the $ref is handled within the composition logic [8][7]. This approach avoids the issue of sibling properties being ignored [6][7]. Note that this restriction on sibling properties is a specific characteristic of OpenAPI 3.0 [5][9]. In OpenAPI 3.1, the handling of $ref evolved to align with JSON Schema 2020-12, where sibling keywords are generally allowed on schema references [10][11].
Citations:
- 1: https://swagger.io/docs/specification/v3_0/using-ref/
- 2: https://spec.openapis.org/oas/v3.0
- 3: https://spec.openapis.org/oas/v3.0.3.html
- 4: https://redocly.com/learn/openapi/ref-guide
- 5: oas core rule: $ref cannot have siblings stoplightio/spectral#394
- 6: Normalize invalid nullable
$refschemas orval-labs/orval#3714 - 7: Reference objects don't combine well with “nullable” OAI/OpenAPI-Specification#1368
- 8: Allow more attributes for "Reference Object" OAI/OpenAPI-Specification#4235
- 9: Why "all other properties in a "$ref" object are ignored"? OAI/OpenAPI-Specification#3087
- 10: Feature Request: Allow sibling elements with $ref that overrides the references definition OAI/OpenAPI-Specification#2744
- 11: Why OpenAPI 3.1.0 does not allow schema references in Parameter & Media Type objects? OAI/OpenAPI-Specification#2676
nullable $ref는 anyOf로 분기하세요.
prefill, estImpressions, estClicks는 필드 문서에서 null을 허용하지만, 현재 allOf + nullable: true는 OpenAPI 3.0에서 null 허용 범위를 만들지 않습니다. OpenAPI 3.0에서는 $ref 주변 nullable 같은 서브 스키마 속성이 동작하지 않으며, allOf의 각 항목은 독립적으로 검증됩니다.
return new ComposedSchema()
.addAnyOfItem(new Schema<>().$ref(property.get$ref()))
.addAnyOfItem(new ObjectSchema().type("null"))
.description(description);NullableSchemaConfig, 관련 DTO, 그리고 OpenApiContractTest의 prefill 계약 검증을 함께 변경하세요.
📍 Affects 4 files
src/main/java/chaeso/zip/server/common/config/NullableSchemaConfig.java#L26-L29(this comment)src/main/java/chaeso/zip/server/auth/application/dto/GoogleAuthResponse.java#L33-L34src/main/java/chaeso/zip/server/simulation/application/dto/SimulationItemResponse.java#L29-L32src/test/java/chaeso/zip/server/docs/OpenApiContractTest.java#L353-L359
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/chaeso/zip/server/common/config/NullableSchemaConfig.java`
around lines 26 - 29, Update NullableSchemaConfig to represent nullable $ref
properties with anyOf containing the referenced schema and a null ObjectSchema,
removing the ineffective allOf/nullable combination while preserving the
description. Apply the related DTO changes in GoogleAuthResponse and
SimulationItemResponse, and update OpenApiContractTest’s prefill contract
assertions to expect the new anyOf nullable schema.
| @Schema(description = "성공 시 응답 본문. 실패 시 null") | ||
| private final T data; | ||
|
|
||
| @Schema(description = "실패 시 에러 정보. 성공 시 생략") | ||
| @Schema(description = "실패 시 에러 정보. 성공 시 null") | ||
| private final ErrorResponse error; | ||
|
|
||
| @Schema(description = "성공 안내 코드. 안내할 것이 없으면 응답에서 생략") | ||
| @Schema(description = "성공 안내 코드. 안내할 것이 없으면 null", | ||
| example = "GOOGLE_ACCOUNT_LINKED") | ||
| private final String code; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
import json
import re
import textwrap
files = [
"src/main/java/chaeso/zip/server/auth/presentation/AuthApiDocs.java",
"src/main/java/chaeso/zip/server/recommendation/presentation/RecommendationApiDocs.java",
"src/main/java/chaeso/zip/server/simulation/presentation/SimulationApiDocs.java",
]
pattern = re.compile(
r'String\s+([A-Z0-9_]+_EXAMPLE)\s*=\s*"""(.*?)""";',
re.DOTALL,
)
for filename in files:
text = Path(filename).read_text()
for name, body in pattern.findall(text):
payload = json.loads(textwrap.dedent(body))
missing = {"data", "error", "code"} - payload.keys()
if missing:
print(f"{filename}:{name}: missing {sorted(missing)}")
PY
rg -n -C 2 '"success":|"data":|"error":|"code":' docs/openapi.mdRepository: YAPP-Github/chaeso-zip-server
Length of output: 4565
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== ApiResponse =="
cat -n src/main/java/chaeso/zip/server/common/response/ApiResponse.java | sed -n '1,180p'
echo
echo "== ResponseWrapperSchemaCustomizer =="
rg -n "ResponseWrapperSchemaCustomizer|`@schema`|schema|required|ApiResponse|data|error|code" src/main/java -g '*.java' | sed -n '1,220p'
echo
echo "== OpenAPI wrapper examples =="
sed -n '90,170p' docs/openapi.md
echo
echo "== SecurityConfig auth failure =="
rg -n -C 4 "AuthenticationFailure|AuthenticationEntryPoint|sendFailed|ApiResponse\.fail|fail\\(" src/main/java/chaeso/zip/server/security src/main/java -g '*.java' | sed -n '1,240p'Repository: YAPP-Github/chaeso-zip-server
Length of output: 44496
ApiResponse 예시와 401 응답을 래퍼 계약에 맞춰 보정하세요.
ApiResponse가 항상 data, error, code를 포함하지만 doc 예시는 종종 생략합니다. ApiResponse.success(...) 예시는 error: null, code: null을, ApiResponse.fail(...) 예시는 data: null, code: null을 추가해 주세요. 이 시점에 401 응답도 CommonResponsesCustomizer가 ApiResponse.fail(error) 예시를 주므로, data: null, code: null을 포함하도록 업데이트하세요.
수정 예시
{
"success": true,
"data": { ... },
+ "error": null,
+ "code": null
} {
"success": false,
+ "data": null,
"error": { ... },
+ "code": null
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/chaeso/zip/server/common/response/ApiResponse.java` around
lines 27 - 35, Update the OpenAPI examples for ApiResponse.success(...) to
explicitly include error and code as null, and for ApiResponse.fail(...) to
include data and code as null. Also update the 401 response example configured
by CommonResponsesCustomizer, which uses ApiResponse.fail(error), to include
data and code as null while preserving the existing error example.
| .andExpect(jsonPath("$.data.logoUrl").value(nullValue())) | ||
| .andExpect(jsonPath("$.data.products[0].pricing[0].valueMax").value(nullValue())) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
ctr도 null 키 계약으로 검증하세요.
Line 224는 fixture에서 null로 전달한 $.data.products[0].ctr가 없어야 한다고 요구합니다. 이 검증은 nullable 응답 필드를 키와 함께 반환하는 PR 계약과 충돌합니다.
doesNotExist()를 value(nullValue())로 교체하세요. 그렇지 않으면 올바른 직렬화가 테스트를 실패시키거나, 잘못된 필드 생략이 테스트를 통과합니다.
수정 예시
- .andExpect(jsonPath("$.data.products[0].ctr").doesNotExist())
+ .andExpect(jsonPath("$.data.products[0].ctr").value(nullValue()))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .andExpect(jsonPath("$.data.logoUrl").value(nullValue())) | |
| .andExpect(jsonPath("$.data.products[0].pricing[0].valueMax").value(nullValue())) | |
| .andExpect(jsonPath("$.data.products[0].ctr").value(nullValue())) | |
| .andExpect(jsonPath("$.data.logoUrl").value(nullValue())) | |
| .andExpect(jsonPath("$.data.products[0].pricing[0].valueMax").value(nullValue())) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/chaeso/zip/server/channel/presentation/ChannelControllerTest.java`
around lines 225 - 226, Update the assertion for $.data.products[0].ctr in
ChannelControllerTest to use value(nullValue()) instead of doesNotExist(), so
the test verifies that the nullable key is present with a null value. Keep the
existing assertions for logoUrl and valueMax unchanged.
PR 리뷰 요약 (CI 자동 리뷰)nullable 필드 표기 전환( 🟠 Major
이 PR로 GoogleAuthResponse, SimulationResponse, SimulationItemResponse, ApiResponse 등에서 @JsonInclude(NON_NULL)이 제거되어, 값이 없는 필드(linkRequired, signupRequired, accessToken, refreshToken, code, simulationId, items, shortfallWon, error 등)도 이제 키가 null로 항상 실립니다. 그런데 위 테스트들은 여전히 jsonPath(...).doesNotExist()로 검증하고 있습니다. Spring의 JsonPathExpectationsHelper.doesNotExist()는 키가 아예 없을 때뿐 아니라 값이 null일 때도 통과하므로, 이 assert들은 옛 계약(생략)이든 새 계약(null로 실림)이든 항상 그린이라 실제로는 이 PR이 바꾼 동작을 전혀 검증하지 못합니다. 게다가 AuthControllerTest.java:376의 DisplayName("구글 로그인 분기는 토큰만 내려주고 분기 플래그를 싣지 않는다")나 SimulationControllerTest.java:213의 주석("값이 없는 선택 필드는 null 로 담지 않고 생략한다. 스키마의 NOT_REQUIRED 와 같은 계약")은 이 PR이 명시적으로 뒤집은 옛 계약을 그대로 서술하고 있어, 6개월 뒤 이 코드를 보는 사람에게 실제 동작과 반대되는 정보를 줍니다. 같은 PR에서 ChannelControllerTest/RecommendationControllerTest에 적용한 value(nullValue()) 패턴으로 통일하고, DisplayName/주석도 "null로 내려간다"로 갱신해 주세요. 이 외에 병합을 막을 만한 문제는 발견하지 못했습니다. |
2 participants
🔗 관련 이슈
📝 작업 내용
REQUIRED+nullable = trueNOT_REQUIREDnull이 아니라[]@JsonInclude(NON_NULL)이나spring.jackson.default-property-inclusion사용 X✅ 체크리스트
Summary by CodeRabbit
변경 사항
null로 반환됩니다.null허용으로 명확히 표시됩니다.data,error,code필드 규칙이 일관되게 정리되었습니다.null반환 사례와 주요 응답 필드가 보강되었습니다.문서
테스트