fix: getNextStringId 전략 인자가 싱글턴 기본 전략을 오염시키는 문제 수정 - #298
Open
z3rotig4r wants to merge 1 commit into
Open
Conversation
getNextStringId(EgovIdGnrStrategy)와 getNextStringId(String)이 인자로 받은 전략을 인스턴스 필드 strategy에 대입해, 임시 전략으로 한 번 호출하면 이후 모든 호출자의 무인자 getNextStringId()가 그 전략을 사용했다. ID 생성 서비스는 싱글턴 빈이므로 호출자 간 상태가 새어나가고, 설정 주입용 setStrategy()가 별도로 있는데 조회 메서드가 상태를 바꾸는 것은 계약에도 어긋난다. 인자로 받은 전략을 지역 변수로만 사용하도록 바꿔 해당 호출에만 적용되게 했다. public 시그니처와 기본 동작은 그대로다. 전략 격리와 setStrategy 무회귀를 검증하는 IdGnrStrategyIsolationTest를 추가했다.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
문제
AbstractIdGnrService의getNextStringId(EgovIdGnrStrategy)와getNextStringId(String)은 인자로 받은 전략을 인스턴스 필드this.strategy에 대입한 뒤 무인자getNextStringId()를 호출합니다. 대입된 전략은 호출이 끝나도 그대로 남습니다. ID 생성 서비스는 보통 싱글턴 빈으로 등록해 여러 업무가 함께 쓰므로, 한 곳에서 전략을 넘겨 한 번 호출하면 그 뒤로는 다른 곳의 무인자getNextStringId()까지 그 전략으로 ID를 만듭니다.기존 테스트에서도 이 오염이 드러납니다. 수정 전 코드로
EgovTableIdGnrServiceTest를 무작위 메서드 순서(MethodOrderer$Random, seed 1)로 실행하면testIdGenStrategy가 실패합니다.앞서 실행된
testNotDefinedTableInfo가 싱글턴 서비스에SMPL-전략을 남긴 결과입니다. 이 테스트가 통과해 온 것은 메서드 실행 순서 덕분입니다.필드 대입 자체도 문제입니다. 임의의 스레드가 런타임에 non-volatile 인스턴스 필드를 쓰는 동작이라, 동시 호출에서 데이터 레이스와 전략 교차 오염이 생깁니다.
수정
두 오버로드에서 필드 대입을 없애고 인자로 받은 전략으로 직접 ID를 만들도록 했습니다.
무인자
getNextStringId()는final이고 본문이strategy.makeId(getNextBigDecimalId().toString())입니다. 기존 코드를 펼치면 수정 후 코드와 글자 그대로 같은 식이고getNextBigDecimalId()호출도 1회로 같습니다. 시퀀스 소비량은 달라지지 않습니다.수정 후에는 이 필드에 대한 런타임 쓰기가 사라지고 설정 시점의
setStrategy()만 남습니다.검증
mvn -B -o -pl Foundation/org.egovframe.rte.fdl.idgnr test로 모듈 테스트 38건이 모두 통과합니다. 무작위 메서드 순서에서도 seed 1·2·3 모두 38건 전부 통과합니다.인자로 넘긴 전략이 서비스 상태를 오염시키지 않는지 검증하는 테스트
IdGnrStrategyIsolationTest3건을 추가했습니다. 전략 객체 인자 호출, 전략 빈 이름 인자 호출,setStrategy()로 지정한 전략이 무인자 호출에 계속 적용되는지를 각각 봅니다. 수정을 되돌리면 앞의 두 건이 실패합니다. 세 번째는setStrategy경로의 무회귀 가드라 수정 전후 모두 통과합니다.영향 범위도 확인했습니다.
strategy필드는 private이고 읽는 곳은getNextStringId()와getStrategy()뿐입니다. 하위 클래스(AbstractDataIdGnrService→AbstractDataBlockIdGnrService→EgovSequenceIdGnrServiceImpl·EgovTableIdGnrServiceImpl)에는 이 필드를 참조하는 코드가 없습니다.EgovUUIdGnrServiceImpl은 이 클래스를 상속하지 않습니다.getNextStringId·getStrategy·setStrategy참조도 idgnr 모듈 밖에는 없습니다.동작 변경 안내
getNextStringId(전략)을 호출한 뒤의 무인자getNextStringId()는 직전에 넘긴 전략을 더 이상 쓰지 않고 서비스에 설정된 기본 전략을 씁니다. 메서드 시그니처와 각 호출이 돌려주는 값은 그대로입니다. 바뀌는 것은 이전 호출이 이후 호출에 남기던 부작용뿐입니다.전략을 계속 적용하려면 원래 그 용도인
setStrategy(EgovIdGnrStrategy)나 Spring 설정의strategy프로퍼티를 쓰면 됩니다. 이 경로가 그대로 동작한다는 것은IdGnrStrategyIsolationTest#testSetStrategyContinuesToApplyToDefaultStringId가 검증합니다.기존 동작에 기대는 코드가 실제로 있기는 어렵습니다. 공통컴포넌트의
getNextStringId호출 181건 중 인자를 넘기는 곳은 테스트용 인터페이스 스텁 2개뿐이고 나머지는 전부 무인자 호출입니다.