Skip to content

Conversation

@gyuwon
Copy link
Contributor

@gyuwon gyuwon commented May 15, 2025

No description provided.

@gyuwon gyuwon requested a review from Copilot May 15, 2025 02:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a test‐only Pbkdf2PasswordEncoder bean and verifies it in integration tests.

  • Define a PasswordEncoderConfiguration class to register a Pbkdf2PasswordEncoder for tests
  • Update the @CommerceApiTest meta‐annotation to include the new configuration
  • Add a new test in CommerceApiApp_specs to assert the PasswordEncoder bean type

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/test/java/test/commerce/api/PasswordEncoderConfiguration.java Introduce a test configuration class providing a password encoder bean
src/test/java/test/commerce/api/CommerceApiTest.java Extend the custom test annotation to load the new config
src/test/java/test/commerce/CommerceApiApp_specs.java Add a test to verify the PasswordEncoder bean is available
Comments suppressed due to low confidence (1)

src/test/java/test/commerce/CommerceApiApp_specs.java:12

  • This test uses @SpringBootTest directly against CommerceApiApp, so it won’t load the PasswordEncoderConfiguration. Either switch to @CommerceApiTest or include PasswordEncoderConfiguration in the classes list.
@SpringBootTest(classes = CommerceApiApp.class)

Comment on lines +3 to +8
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Primary;
import org.springframework.security.crypto.password.Pbkdf2PasswordEncoder;

import static org.springframework.security.crypto.password.Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256;

Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class is not annotated with @configuration or @TestConfiguration, so its @bean method won’t be picked up. Add @TestConfiguration to register the bean in the test context.

Suggested change
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Primary;
import org.springframework.security.crypto.password.Pbkdf2PasswordEncoder;
import static org.springframework.security.crypto.password.Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256;
import org.springframework.context.annotation.Bean;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.context.annotation.Primary;
import org.springframework.security.crypto.password.Pbkdf2PasswordEncoder;
import static org.springframework.security.crypto.password.Pbkdf2PasswordEncoder.SecretKeyFactoryAlgorithm.PBKDF2WithHmacSHA256;
@TestConfiguration

Copilot uses AI. Check for mistakes.
Comment on lines 10 to 14

@Bean
@Primary
Pbkdf2PasswordEncoder testPasswordEncoder() {
return new Pbkdf2PasswordEncoder("", 16, 10, PBKDF2WithHmacSHA256);
Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Extract the magic numbers (salt length 16 and iteration count 10) into named constants or add comments to clarify their purpose.

Suggested change
@Bean
@Primary
Pbkdf2PasswordEncoder testPasswordEncoder() {
return new Pbkdf2PasswordEncoder("", 16, 10, PBKDF2WithHmacSHA256);
private static final int SALT_LENGTH = 16; // Length of the salt in bytes
private static final int ITERATION_COUNT = 10; // Number of hash iterations
@Bean
@Primary
Pbkdf2PasswordEncoder testPasswordEncoder() {
return new Pbkdf2PasswordEncoder("", SALT_LENGTH, ITERATION_COUNT, PBKDF2WithHmacSHA256);

Copilot uses AI. Check for mistakes.
@gyuwon gyuwon merged commit 691d11d into main May 15, 2025
1 check passed
@gyuwon gyuwon deleted the test-perf branch May 15, 2025 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants