Skip to content
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

Refactor : Adjust resolveFieldName Method Logic to Correctly Handle Primitive boolean Field Names #1096

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ public final class JavaGetterPropertyFieldNameResolver {

@Nullable
public String resolveFieldName(Class<?> targetClass, String methodName) {
if (hasPrefix(GET_PREFIX, methodName)) {

if (isValidField(targetClass, methodName)) {
Copy link
Contributor

@seongahjo seongahjo Nov 20, 2024

Choose a reason for hiding this comment

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

하위호환을 보장하고 안전하게 변경됨을 보장하기 위해 다음 테스트들이 추가되면 좋을 것 같습니다.

  1. is가 이름의 prefix로 붙은 boolean이 아닌 필드를 set
  2. is가 이름의 prefix로 붙지않은 boolean의 필드를 set
  3. is가 이름의 prefix로 붙은 boolean인 필드를 set
  4. is가 이름의 prefix로 붙지않은 boolean이 아닌 필드를 set

Copy link
Author

@rawfishthelgh rawfishthelgh Nov 25, 2024

Choose a reason for hiding this comment

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

테스트 추가했습니다!

  1. is가 이름의 prefix로 붙은 boolean이 아닌 필드를 set
  2. is가 이름의 prefix로 붙지않은 boolean의 필드를 set
  3. is가 이름의 prefix로 붙은 boolean인 필드를 set
  4. is가 이름의 prefix로 붙지않은 boolean이 아닌 필드를 set

추가로 Boolean 필드가 Wrapper Type일 경우와 Primitive Type일 경우의 테스트도 작성했습니다.

// class could be using property-style getters (e.g. java record)
return methodName;
} else if (hasPrefix(GET_PREFIX, methodName)) {
return stripPrefixPropertyName(targetClass, methodName, GET_PREFIX.length());
} else if (hasPrefix(IS_PREFIX, methodName)) {
return stripPrefixPropertyName(targetClass, methodName, IS_PREFIX.length());
} else if (isValidField(targetClass, methodName)) {
// class could be using property-style getters (e.g. java record)
return methodName;
}

return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Fixture Monkey
*
* Copyright (c) 2021-present NAVER Corp.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.navercorp.fixturemonkey.tests.java;

import static com.navercorp.fixturemonkey.api.expression.JavaGetterMethodPropertySelector.javaGetter;
import static com.navercorp.fixturemonkey.tests.TestEnvironment.TEST_COUNT;
import static org.assertj.core.api.BDDAssertions.thenCode;

import org.junit.jupiter.api.RepeatedTest;

import lombok.AllArgsConstructor;
import lombok.Getter;

import com.navercorp.fixturemonkey.FixtureMonkey;
import com.navercorp.fixturemonkey.api.introspector.ConstructorPropertiesArbitraryIntrospector;

class JavaGetterPropertyFieldNameResolverTest {

private static final FixtureMonkey SUT = FixtureMonkey.builder()
.objectIntrospector(ConstructorPropertiesArbitraryIntrospector.INSTANCE)
.build();

@RepeatedTest(TEST_COUNT)
void testNonBooleanFieldWithIsPrefix() {
thenCode(SUT.giveMeBuilder(JavaGetterObject.class)
.set(javaGetter(JavaGetterObject::getIsStatus), "javaGetterStringStatus")::sample)
Copy link
Contributor

Choose a reason for hiding this comment

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

set 같은 연산이 정상적으로 동작했는지 검증하는 게 좋을 것 같습니다.

ex. then(actual.getIsStatus()).isEqualTo("javaGetterStringStatus")

.doesNotThrowAnyException();
}

@RepeatedTest(TEST_COUNT)
void testPrimitiveTypeBooleanFieldWithIsPrefix() {
thenCode(SUT.giveMeBuilder(JavaGetterObject.class)
.set(javaGetter(JavaGetterObject::isActive), true)::sample)
.doesNotThrowAnyException();
}

@RepeatedTest(TEST_COUNT)
void testBooleanFieldWithoutIsPrefix() {
thenCode(SUT.giveMeBuilder(JavaGetterObject.class)
.set(javaGetter(JavaGetterObject::isEnabled), true)::sample)
.doesNotThrowAnyException();
}

@RepeatedTest(TEST_COUNT)
void testNonBooleanFieldWithoutIsPrefix() {
thenCode(SUT.giveMeBuilder(JavaGetterObject.class)
.set(javaGetter(JavaGetterObject::getName), "javaGetterObjectName")::sample)
.doesNotThrowAnyException();
}

@RepeatedTest(TEST_COUNT)
void testWrapperTypeBooleanFieldWithIsPrefix() {
thenCode(SUT.giveMeBuilder(JavaGetterObject.class)
.set(javaGetter(JavaGetterObject::getIsDeleted), true)::sample)
.doesNotThrowAnyException();
}

@AllArgsConstructor
@Getter
private static class JavaGetterObject {
private String isStatus;
private boolean isActive;
private boolean enabled;
private String name;
private Boolean isDeleted;
}
}
Loading