Maintenance: SwaggerConfig - Add environment-aware server configuration

[claude]

Summary

Improved SwaggerConfig to provide environment-aware server configuration and enhanced API documentation.

Area Inspected

• src/main/java/spring/memewikibe/config/SwaggerConfig.java

Issues Found

1. Hardcoded Production Server URL: The configuration only contained a hardcoded production server URL (https://api.meme-wiki.net), making it inconvenient for local development and testing where developers need to test against localhost:8080.

2. No Environment Adaptation: Despite having multiple Spring profiles (local, dev, prod) in application.yml, the Swagger configuration didn't adapt to different environments.

3. Minimal API Description: The API description was very basic and didn't provide users with information about available features.

4. Code Organization: All configuration was inline in a single method, making it harder to read and maintain.

Changes Made

1. Environment-Aware Server Configuration:

   • Added @Value injection to read the active Spring profile
   • Created buildServers() method that returns appropriate server URLs based on the profile:
     • local: localhost:8080 only
     • dev: both production and localhost URLs for flexibility
     • prod: production URL only
   • This improves developer experience by automatically showing the correct server in Swagger UI

2. Enhanced API Documentation:

   • Extracted API info building to buildApiInfo() method
   • Added comprehensive description using Java 21 text blocks
   • Listed key features (search, categories, sharing, recommendations, quiz)

3. Better Code Organization:

   • Separated concerns into buildApiInfo() and buildServers() helper methods
   • Improved readability and maintainability
   • Used modern Java 21 features (text blocks, switch expressions)

Why These Changes Improve the Code

1. Developer Experience: Developers no longer need to manually change the server URL in Swagger UI when testing locally - it automatically uses localhost:8080 in local profile.

2. Documentation Quality: API consumers get better context about what the API offers directly in the Swagger UI.

3. Maintainability: Separated methods make it easier to modify server configuration or API info independently.

4. Consistency: Follows the same pattern as other config files in the codebase that adapt to different profiles.

5. Modern Java: Uses Java 21 features (text blocks for multiline strings, switch expressions) consistent with the project's Java 21 requirement.

Testing

• ✅ All tests pass (./gradlew test)
• ✅ Build succeeds (./gradlew build)
• ✅ No breaking changes - purely additive improvements

🤖 Generated with Claude Code

[claude]

Code Review for PR #126: SwaggerConfig Environment-Aware Configuration

Overall Assessment

This is a well-intentioned improvement to make Swagger documentation environment-aware. However, there are several critical issues that need to be addressed before merging.

---

🔴 Critical Issues

1. Incorrect Profile Injection Pattern (Line 16)

File: src/main/java/spring/memewikibe/config/SwaggerConfig.java:16

@Value("${spring.profiles.active:local}")
private String activeProfile;

Problem:

• spring.profiles.active is not a single property when multiple profiles are active. Spring Boot stores this as an array/list.
• If multiple profiles are active (e.g., local,dev), this will fail or return unexpected values.
• The field injection happens after construction, so there's a potential timing issue with the bean creation lifecycle.

Recommendation:
Use Spring's Environment abstraction instead:

private final Environment environment;

public SwaggerConfig(Environment environment) {
    this.environment = environment;
}

// In buildServers():
String[] activeProfiles = environment.getActiveProfiles();
String profile = activeProfiles.length > 0 ? activeProfiles[0] : "local";

Or use @Profile annotation to create profile-specific beans:

@Bean
@Profile("prod")
public OpenAPI prodOpenAPI() {
    return new OpenAPI()
        .info(buildApiInfo())
        .servers(List.of(new Server()
            .url("https://api.meme-wiki.net")
            .description("Production Server")));
}

@Bean
@Profile({"local", "default"})
public OpenAPI localOpenAPI() {
    // ... local configuration
}

---

2. Inconsistent Server URLs Between dev and prod (Lines 52-58)

File: src/main/java/spring/memewikibe/config/SwaggerConfig.java:52-58

case "dev" -> {
    servers.add(new Server()
        .url("https://api.meme-wiki.net")  // Same as prod!
        .description("Development Server"));
    servers.add(new Server()
        .url("http://localhost:8080")
        .description("Local Development"));
}

Problem:

• The dev profile uses the same production URL (https://api.meme-wiki.net) as the prod profile.
• This is confusing and potentially dangerous - developers might accidentally hit production when they think they're in dev mode.
• Based on the application.yml (lines 92-108), the dev profile has its own database (DATABASE_DEV_NAME), suggesting it should have its own environment.

Recommendation:
Either:

1. Use a separate dev server URL (e.g., https://dev-api.meme-wiki.net)
2. Or clarify in comments why dev uses the production URL
3. Consider if dev profile is actually meant to be a staging environment

---

⚠️ Major Issues

3. Missing Test Coverage

Issue: No tests exist for this configuration class.

Why This Matters:

• The profile-based logic has multiple branches that should be tested
• Configuration beans are critical infrastructure code
• Refactoring without tests is risky

Recommendation:
Add test coverage:

@SpringBootTest
class SwaggerConfigTest {
    
    @Test
    @ActiveProfiles("prod")
    void shouldConfigureProdServerOnly() {
        // Test that only production server is configured
    }
    
    @Test
    @ActiveProfiles("local")
    void shouldConfigureLocalServerOnly() {
        // Test that only localhost is configured
    }
    
    @Test
    @ActiveProfiles("dev")
    void shouldConfigureDevAndLocalServers() {
        // Test that both servers are configured
    }
}

---

4. Switch Expression Without Exhaustive Cases (Lines 48-65)

File: src/main/java/spring/memewikibe/config/SwaggerConfig.java:48-65

Problem:

• The switch uses default for the local profile, but this means any unknown profile will also get the local configuration.
• If someone creates a test or staging profile, they'll unexpectedly get localhost configuration.

Recommendation:
Be explicit:

switch (activeProfile) {
    case "prod" -> servers.add(new Server()
        .url("https://api.meme-wiki.net")
        .description("Production Server"));
    case "dev" -> {
        servers.add(new Server()
            .url("https://api.meme-wiki.net")
            .description("Development Server"));
        servers.add(new Server()
            .url("http://localhost:8080")
            .description("Local Development"));
    }
    case "local" -> servers.add(new Server()
        .url("http://localhost:8080")
        .description("Local Development Server"));
    default -> {
        // Log warning for unknown profile
        servers.add(new Server()
            .url("http://localhost:8080")
            .description("Unknown Profile - Defaulting to Local"));
    }
}

---

💡 Minor Issues / Suggestions

5. Immutability Pattern Not Followed

File: src/main/java/spring/memewikibe/config/SwaggerConfig.java:16

Observation:

• The class uses field injection with a mutable field activeProfile
• Modern Spring best practices favor constructor injection with final fields

Recommendation:

private final String activeProfile;

public SwaggerConfig(@Value("${spring.profiles.active:local}") String activeProfile) {
    this.activeProfile = activeProfile;
}

---

6. Unnecessary ArrayList Creation (Line 45)

File: src/main/java/spring/memewikibe/config/SwaggerConfig.java:45

List<Server> servers = new ArrayList<>();

Issue:

• You're creating a mutable ArrayList but the number of servers is known at compile time.
• In most cases, you're adding 1-2 servers.

Recommendation:
Use List.of() or pre-sized ArrayList:

// For single server cases:
return List.of(new Server()...);

// If you need mutability:
List<Server> servers = new ArrayList<>(2); // Pre-sized for efficiency

---

7. Text Block Indentation (Lines 30-41)

File: src/main/java/spring/memewikibe/config/SwaggerConfig.java:30-41

Minor: The text block is well-formatted, but ensure the indentation matches your team's style guide.

---

8. Security Consideration: Swagger in Production

File: src/main/java/spring/memewikibe/config/SecurityConfig.java:32

Observation:
Looking at SecurityConfig.java, Swagger UI is publicly accessible in all profiles:

.requestMatchers("/api/**", "/health", "/swagger-ui/**", "/v3/api-docs/**").permitAll()

Recommendation:
Consider restricting Swagger UI in production:

• Either disable it entirely in prod profile
• Or require authentication to access it
• Document this decision if it's intentional

---

📊 Code Quality

Positives ✅

1. Good code organization - Separation into buildApiInfo() and buildServers() methods
2. Uses Java 21 features - Text blocks and switch expressions are appropriate
3. Descriptive API documentation - The Korean description is comprehensive
4. Clear PR description - Good explanation of the changes and motivation

Areas for Improvement 📈

1. No logging - Consider adding debug logs for which profile is active
2. No validation - No check if the profile value is valid
3. Magic strings - Profile names are hardcoded; consider constants
4. No javadoc - Public methods should have documentation

---

🔐 Security Assessment

No significant security vulnerabilities introduced.

However, note:

• Swagger UI exposes API structure publicly (as configured in SecurityConfig)
• Production server URL is publicly visible (this is expected for public APIs)

---

🚀 Performance Assessment

No performance concerns.

The configuration is loaded once at application startup. The switch expression and text blocks have no runtime performance impact.

---

📝 Recommendations Summary

Must Fix Before Merge:

1. ✅ Fix the profile injection pattern (use Environment instead of @Value)
2. ✅ Clarify/fix the dev server URL inconsistency
3. ✅ Add test coverage for profile-based configuration

Should Fix:

4. Make the switch expression exhaustive with explicit cases
5. Use constructor injection instead of field injection

Nice to Have:

6. Add logging for debugging
7. Use List.of() for immutable server lists
8. Add javadoc comments
9. Consider profile name constants

---

🎯 Verdict

Changes Requested - The core idea is good, but the implementation has critical issues with profile handling that could cause runtime failures or unexpected behavior. Once the profile injection is fixed and tests are added, this will be a solid improvement.

Estimated Effort to Fix: 1-2 hours

• Fix profile injection: 15 minutes
• Fix dev URL: 5 minutes
• Add tests: 45-60 minutes

---

Review conducted by Claude Code