FindBugs 적용 후 흔히 발생하는 버그 패턴들 프로그래밍

회사의 모든 프로젝트는 FindBugs 지표 수집을 하게 돼 있는데 이를 SonarQube를 통해 프로젝트 건강성 지표로만 삼고 있었다(findbugs, jacoco code coverage, checkstyle, pmd 등을 수집한다).
하지만 지표는 참조일뿐 강제가 아니고, 바쁘면 바쁘다는 핑계로 무시하게 마련이다.

그러다가 팀원들과 팀 프로젝트에서 FindBugs 의 버그 탐지를 단순 지표가 아닌 build 실패로 간주하도록 (빌드가 실패하면 당연히 배포 불가이다) 하기로 논의하여 결정하고 적용했더니 매주 1~2건씩은 배포 전에 버그를 탐지해 막아주고 있다(물론 이 버그들은 코드 Coverage 100%의 테스트를 했다면 발생하지 않았겠지만 그건 다음에...).
물론 Build 행위는 Jenkins + Gradle 조합으로 이뤄지며 Jenkins의 FindBugs 플러그인으로 FindBugs 의 버그가 탐지되면 빌드 실패로 간주하고 배포를 불가능하게 설정하였다.

FindBugs는 가급적 단순 지표가 아닌 실제 배포 가능 여부를 가늠하는 도구로써 버그 탐지시에는 실질적으로 컴파일이 안 된 것과 마찬가지로 취급하는 것이 좋겠다.

지금까지 내가 본 흔히 나오는 버그 패턴은 다음과 같다(컴파일은 되지만 100% 버그이므로 따라하면 안됨!!).

1. null 체크해놓고 그 객체 호출하기(보통은 logging 시에 실수로 호출함)
if (a == null) {
....
    log.debug("a.something {}", a.getSomething()); // 로깅 코드에서 null pointer exception이 발생해서 로그도 안남고 에러가 남.
}


2. 서로 다른 타입간의 equals 호출
Integer a = ..;
Long b = ..;
if (a.equals(b)) { // 언제나 false
...
}


또한 흔한게 enum 과 String 비교.
String enumValue = "..";
if (SomeEnum.ENUMVALUE.equals(enumValue)) { // 언제나 false
...
}


enum은 아예 equals()를 사용하지 말고 == 비교를 하는 게 낫다.

3. 숫자 Wrapper 객체 동일성 == 비교
Integer a = new Integer(1);
Integer b = new Integer(1);
if (a == b) { ... } // false이다.


Wrapper 객체는 equals() 로 비교해야 한다.

4. List/Map 등 컬렉션에서 엉뚱한 객체로 get 하기
안타깝게도 List.indexOf()와 Map.get()은 인자로 지정된 Generic Type을 받는게 아니라 Object를 받는다. 이로 인해 버그가 발생한다(이렇게 한 이유가 있겠지...).
List<Integer> integers = ...;
Long someLongValue = 1L;

// Integer 1 값이 리스트에 있어도 올바로 판단 못함
if (integers.indexOf(someLongValue) >= 0) { // false이다.
...
}


Map.get()도 마찬가지이다. 특히 이 경우의 버그가 많이 발생한다.
Map<Integer,String> map = new HashMap<>();
map.put(1, "하나");
map.put(2, "둘");

System.out.println(map.get(1L)); // null


예제에서는 단순화하려고 값을 직접 넣었지만 대부분은 메소드 파라미터로 받았기 때문에 코드의 호출부에서는 파라미터의 타입이 눈에 안 띄어서 버그여부를 인지하기가 쉽지 않다.

위에서 보다시피가장 많은 버그는 메소드의 인자가 Object 일 때 발생한다(equals, List.indexOf, Map.get). 컴파일 에러가 안나기 때문이다.

FindBugs 가 정착이 다 되면, 이후에는 다음과 같은 것도 해볼까 생각중.
  1. Jenkins JaCoCo Plugin 을 통해 현 시점의 코드 커버리지를 임계치로 설정하고 임계치보다 떨어지면 빌드 실패하게, 그리고 1주일에 1%씩 임계치 증가시키키(아무튼 최소한 커버리지를 떨어뜨리지는 못하게)
  2. Jenkins CheckstyleJenkins PMD 적용하고 임계치를 서서히 증가시키기

위 툴들은 우리회사에서는 이미 SonarQube를 통해 지표 수집은 다 하고 있는 것들이다. 거기에 강제성을 부여하는 것만 더 하고 싶은 것.


마지막으로.. FindBugs가 버그를 찾아줬으니 버그가 없다는 착각 따위는 하지 말 것. 결국은 열심히 단위 테스트를 해야 한다..

단점 : 빌드 시간이 너무 길어져서 배포 시간이 증가했다... ㅜㅜ


동적 Native SQL 생성 어떻게 할까 - 순수 Java 코드로 생성하기 프로그래밍

앞서 Freemarker Dynamic QL Builder에 이어, 순수 Java 코드로 동적 Native SQL/JPQL/HQL을 생성하는 방법에 대한 고민의 결과를 이제서야 공유한다.

나는 어쩔 수 없이 동적 NativeSQL을 작성해야하는 상황에서라도 MyBatis/iBatis나 Freemarker 같은 외부 템플릿을 사용하는 것을 좋아하지 않는 편이다.

일단 로직이 없는 정적 SQL의 경우 템플릿을 사용할 이유가 없고, 로직이 존재하는 SQL은 템플릿에서 처리 할 수 없는 로직이 분명히 존재하기 때문에 템플릿과 Java 코드간의 로직 분할이 발생하게 되고 이로인해 로직을 수정해야 할 때 둘 중 한 곳을 간과하여 버그를 만들기 쉬워진다. 또한 Code Coverage 측정이 안 되어 코드에 대한 확신감도 떨어진다.

MyBatis를 사용한다 해도 가급적 SQL Builder 클래스를 통해 Java 기반으로 쿼리를 생성하라고 권하고 싶다(물론 이 경우 DBA와의 협업이 어려워지는데, 솔직히 코드 작성하면서 DBA에게 MyBatis XML 얼마나 보여주는지 생각해보면 거의 없을 것이다).

다음과 같은 순서로 살펴본다.

  1. java.util.StringBuilder를 사용하는 기본적인 방법을 알아보고,
  2. StringBuilder 기반으로 string-builder-ql-params를 통해 좀 더 쉽게 하는 방법
  3. 아예 StringBuilder를 개선한 UnderscoreStringBuilder라는 것을 통해 처리하는 방법


3번은 사족같은 것이고, 2번까지만 봐도 도움이 많이 될 것 같다.

1. StringBuilder로 동적 SQL만들기


지난 글에서 말했듯이 동적 SQL 생성의 핵심은 동적으로 쿼리 문자열을 만드는데 있는 것이 아니라 그렇게 만들어진 쿼리에 파라미터를 바인딩(binding)하는 것에 있다.

동적 문자열 생성은 StringBuilder 만으로도 충분하다. 여기에 보태어 순서대로 쿼리 파라미터 객체를 저장할 List<Object> params 객체가 있으면 된다. 아래와 같은 형태가 될 것이다.

먼저 아래와 같은 데이터 객체들이 존재한다고 하자.
User user = new User();
// User.COLUMNS
public static final String[] COLUMNS = new String[]{"user_id", "name", "email", "birthday", "mobile_phone", "home_phone", "address", "zip_code"};

user.setUserId(10001L);
user.setName("UnderscoreQlParams");
user.setBirthday(new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/11"));
user.setEmail("someone@email.com");

List<String> zipCodes  = Arrays.asList("12345", "56789", "58391");

이제 이를 가지고 쿼리를 생성하면,
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
import static org.apache.commons.lang3.StringUtils.join;

StringBuilder sb = new StringBuilder();
List<Object> params = new ArrayList<Object>();
sb
  .append("SELECT ")
  .append(join(User.COLUMNS, ", "))
  .append("\n")
  .append("FROM users as u\n");

sb.append("WHERE 1 = 1"); // BUG!! no spaces.
if (user.getUserId() != null) {
  sb.append("AND user_id = ? \n");
  params.add(user.getUserId());
}
if (isNotEmpty(user.getName())) {
  sb.append("AND name = ? \n");
  params.add(user.getName());
}
if (user.getBirthday() != null) {
  sb.append("AND birthday = ? \n");
  params.add(user.getBirthday());
}
// IN Parameter 생성
if (CollectionUtils.isNotEmpty(zipCodes)) {
  List<String> inParams = new ArrayList<String>(zipCodes.size());
  for (int i = 0; i < zipCodes.size(); i++) {
    inParams.add("?");
  }

  sb.append(String.format("AND zip_code IN (%s)", StringUtils.join(inPar",")));
  params.addAll(zipCodes);

}

sb.append("LIMIT 10");


List 하나만 추가해도 그런대로 괜찮은 동적 SQL 생성이 가능해진다. sb.toString()으로 동적으로 생성된 SQL문자열을 가져다가 params에 저장된 각 파라미터 객체를 PreparedStatement.setObject(index, value)(index는 1부터시작)로 저장만 해주면된다. 지금까지 문자열을 더해가며(+) 작업했다면 위 방식으로만 해도 어느정도 괜찮아 진다.

하지만 위에서 굵은 글씨체로 된 부분을 보면 뭔가 문제가 있음을 알 수 있다.

  1. Java Code 사이에서 SQL의 가독성이 떨어지다보니 공백 추가를 잠시 잊을 경우 곧바로 잘못된 SQL을 생성하게 된다. 위 쿼리의 경우 조건에 따라 WHERE 1 = 1AND user_id = ? 이렇게 공백없이 붙어버린 잘못된 쿼리가 생성된다.
  2. WHERE 조건에 대해 언제 AND, OR가 올지 몰라 1 = 1로 방어해줘야 한다. 하지만 대부분의 DB는 최적화가 잘 돼서 이로인한 성능저하는 없다고한다. 따라서 이 문제는 사실상 무시해도 된다.
  3. IN조건의 파라미터를 생성하는 것이 굉장히 복잡하다.

이제 여기서 첫번째와 두번째 문제는 마지막에 알아보고, 세번째 문제를 해결해보자.
그리고 IN 조건절의 경우 Java 8에서 좀 더 간결하게 만들 수도 있는데, 그에 대해서는 논외로한다.

2. StringBuilder 기반으로 string-builder-ql-params를 통해 좀 더 쉽게 하는 방법


위 코드의 List<Object> params에 파라미터를 추가하고 더불어 IN 조건절의 문자열과 파라미터 추가를 좀 더 쉽게 할 수 있는 도우미 클래스를 만들면 쉽게 해결 가능하다. string-builder-ql-params라는 프로젝트에 만들어서 Maven Repository에 올려두었는데, 이 프로젝트 전체를 사용할 필요까진 없고 그냥 DynamicQlParams.java클래스 한개만 복사해 자기 프로젝트에 넣고 사용해도 된다.
DynamicQlParams는 그 안에 파라미터 List객체와 param(Object param), inParams(Iterable|Object[]), bindParameters(PreparedStatement),getParameters()등이 있다.

이 클래스와 StringBuilder를 조합하면 다음과 같이 된다.

import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import static java.lang.String.format;

StringBuilder builder = new StringBuilder();
DynamicQlParams dqp = new DynamicQlParams();

builder
  .append("SELECT ")
  .append(StringUtils.join(User.COLUMNS, ", ")).append("\n")
  .append("FROM users as u\n")
  .append("WHERE 1 = 1\n");

if (user.getUserId() != null) {
  builder.append(format("AND user_id = %s %n", dqp.param(user.getUserId())));
}
if (StringUtils.isNotEmpty(user.getName())) {
  builder.append(format("AND name = %s %n", dqp.param(user.getName())));
}
if (user.getBirthday() != null) {
  builder.append(format("AND birthday = %s %n", dqp.param(user.getBirthday())));
}
if (CollectionUtils.isNotEmpty(zipCodes)) {
  builder.append(format("AND zip_code in (%s) %n",dqp.inParams(zipCodes)));
}

builder.append("LIMIT ").append(dqp.param(10));

PreparedStatement preparedStatement = conn.prepareStatement(builder.toString());
dqp.bindParameters(preparedStatement); // bind parameters to preparedStatement


DynamicQlParamsString.format을 조합했더니 좀 더 가독성 좋고 편리하게 동적 SQL을 생성하게 되었다.
이것 말고도 DynamicNamedQlParams도 만들었다. Spring JDBCTempalte과 JPQL, HQL은 Named Parameter를 지원하기 때문이 이를 사용할 수도 있다.
하지만 Named Parameter는 그 자체가 정적 SQL에서 편리한 파라미터 바인딩을 위한 것이지 동적 SQL에서는 불필요한 기능으로 보인다.

여기서도 사실 구문 사이사이의 공백을 하나라도 잊어버렸을 때의 잘못된 SQL을 생성하는 문제는 해결이 안된 상태이다. 그리고 가독성도 조금만 더 높였으면 좋겠다.

3. StringBuilder를 개선한 UnderscoreStringBuilder를 사용해보기


사실 내가 보기엔 나처럼 정말 어쩌다가 한 두번 동적 NativeSQL을 생성하는 사람은 2번째 방법만 사용해도 괜찮아 보인다. 그래도 학습삼아 좀 오버를 해보았다. 아예 StringBuilder를 대체하는 UnderscoreStringBuilder라는 것을 만들고 거기에 동적 Native SQL생성시 StringBuilder가 가진 단점들들 커버해 줄 수 있는 기능을 넣은 것이다.
아래와 같이 의존성을 추가하고 살펴보도록 하자.
compile '"kr.pe.kwonnam.underscorebuilder:underscore-ql-params:0.1"


이 프로젝트는 딱 두 개의 핵심 클래스로 이뤄진다. StringBuilder를 대체하는 UnderscoreStringBuilder와, 2번에서 DynamicQlParams의 역할을 하는 UnderscoreQlParams 클래스이다.

StringBuilder.append() 역할을 하는 UnderscoreStringBuilder의 핵심 메소드는 __ 그 중에서도 아래 메소드 이다.
UnderscoreStringBuilder.__(boolean appendable, A appendee,
   UnderscoreTransformer<A> transformer, UnderscoreTransformer<? super CharSequence>... extraTransformers)

왜 이름이 UnderscoreStringBuilder인지 알 것 같다. append()보다는 가독성이 좋을 것 같아서 이렇게 했는데, 실제로 그런지는 잘 모르겠다.

이 메소드에서 appendable,transformer,extraTransformers는 모두 생략가능하다.
  • appendable : appendee를 추가할지 여부
  • appendee : 문자열에 추가할 객체. 내부적으로 StringBuilder.append() 호출함.
  • transformer,extraTransformers : appendee를 추가하기 전에 변환 작업을 한다. 지정된 transformer의 순서대로 연달아 변환한다.


UnderscoreQlParams는 여기서 동적 쿼리 생성으로 변환하는 transformer 객체를 생성하고 상태를 저장하는 역할을 한다. 일단 UnderscoreQlParams없이 순수 문자열 기반으로 쿼리를 생성하면 다음과 같은 형태가 된다.
import static kr.pe.kwonnam.underscore.stringbuilder.UnderscoreStringBuilderTransformers.*;
import static kr.pe.kwonnam.underscore.stringbuilder.transformers.trim.TrimOpts.trimOpts;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;

UnderscoreStringBuilder = usb = new UnderscoreStringBuilder();
final SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
usb
    .__("SELECT ").suffixNewLine()
    .__(", ", join(User.COLUMNS))
    .__("FROM users as u")
    .sub(user != null, new UnderscoreSubBuild() {
        @Override
        public void subbuild(UnderscoreStringBuilder underscoreSubBuilder) {
            underscoreSubBuilder.prefix("\n    ")
                .__(user.getUserId() != null, "AND user_id = %d", format(user.getUserId()))
                .__(isNotEmpty(user.getName()), "AND name = '%s'", format(user.getName()))
                .__(user.getBirthday() != null, "AND birthday = '%s'", format(sdf.format(user.getBirthday())))
                .__(CollectionUtils.isNotEmpty(zipCodes), ", ", join(zipCodes), wrap("AND zip_code in (", ")"));

        }
    }, trim(trimOpts().prefix("WHERE ").prefixOverrides("AND ", "OR ")))
    .__("LIMIT 10");


UnderscoreStringBuilderTransformers는 각종 UnderscoreTransformer 객체를 쉽게 생성해주는 static method가 있는 클래스이다. 미리 준비한 transformer들은 format, dateFormat, join, wrap, multiply, trim등이다.
위 코드는 파라미터 바인딩이 필요없는 일반 SQL 문자열을 생성한 것이고, 진짜 목표인 ? 혹은 ?1,?2,... 기반의 PreparedStatement 지원 동적 SQL을 생성해보면 다음과 같은 형태가 된다.
import static kr.pe.kwonnam.underscore.stringbuilder.UnderscoreStringBuilderTransformers.join;
import static kr.pe.kwonnam.underscore.stringbuilder.UnderscoreStringBuilderTransformers.trim;
import static kr.pe.kwonnam.underscore.stringbuilder.transformers.trim.TrimOpts.trimOpts;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;

UnderscoreStringBuilder usb = new UnderscoreStringBuilder();
UnderscoreQlParams qlParams = new UnderscoreQlParams();
usb
    .__("SELECT ").suffixNewLine()
    .__(", ", join(User.COLUMNS))
    .__("FROM users as u")
    .sub(user != null, new UnderscoreSubBuild() { // user != null일 때만 아래 블럭이 실행되고 문자열과 파라미터 추가됨.
        @Override
        public void subbuild(UnderscoreStringBuilder underscoreSubBuilder) {
            underscoreSubBuilder.prefix("\n   ")
                .__(user.getUserId() != null, "AND user_id = %s", qlParams.params(user.getUserId()))
                .__(isNotEmpty(user.getName()), "AND name = %s", qlParams.params(user.getName()))
                .__(user.getBirthday() != null, "AND birthday = %s", qlParams.params(user.getBirthday()))
                .__(CollectionUtils.isNotEmpty(zipCodes), "AND zip_code in (%s)", qlParams.inParams(zipCodes));
        }
    }, trim(trimOpts().prefix("WHERE ").prefixOverrides("AND ", "OR ")))
    .__("LIMIT %s", qlParams.params(10));

log.info("UnderscoreStringBuilder with UnderscoreQlParams : {}", usb.toString()); // 생성된 SQL
log.info("Query Parameters : {}", qlParams.getQueryParameters()); // 바인딩할 Parameters


params, inParams는 내부적으로 String.format을 호출한다. 따라서 앞에 추가할 문자열도 포맷팅 문자열로 만들어줘야 한다. 간단하다. 파라미터 바인딩 물음표가 들어갈 부분을 %s로 해주기만 하면 된다.

UnderscoreStringBuilder.prefix()|suffix()prefixOff()|suffixOff()가 호출될 때까지 모든 문자열 덧붙이기 할 때마다 지정된 문자열을 앞/뒤에 붙여준다. 이를 통해 공백을 까먹어서 생기는 문제를 해소해준다.
그리고 trim transformer를 통해 MyBatis <trim>와 동일한 효과를 낼 수 있게 하였다.
sub 부분은 Java 8 Lambda를 사용하면 좀 더 간결해진다.

지금까지 생성된 쿼리를 보여준 적이 없는데, log 출력 내용은 다음과 같다.

UnderscoreStringBuilder with UnderscoreQlParams : SELECT user_id, name, email, birthday, mobile_phone, home_phone, address, zip_code
FROM users as u
WHERE user_id = ?
   AND name = ?
   AND birthday = ?
   AND zip_code in (?, ?, ?)
LIMIT ?

Query Parameters : [10001, UnderscoreQlParams, Fri Dec 11 00:00:00 KST 2015, 12345, 56789, 58391, 10]


코드를 보면 맨 위에서 도출했던 세가지 문제가 일단은 다 해결된 것을 볼 수 있다.

JPQL/HQL의 ?1, ?2, .. 형태로 쿼리를 생성하고자 한다면 UnderscoreQlParams.withPositionalIndex()를 사용하면 된다.

사실 다 만들어놓고 보니 굳이 이렇게까지 해야하나 싶긴한데, 만드는 과정 자체가 학습이 많이 되어서 만족한다.

1 2 3 4 5 6 7 8 9 10 다음