#164 서비스 약관 동의 API 구현

[yoonseo8167]

• 유저 약관 동의
• 작가 약관 동의
• 약관 리스트 조회
  closed 서비스 약관 동의 api 구현 #164

[Wo-ogie]

id 필드를 Long? type으로 nullable하게 선언하신 건 DB 인입 전에 null 일 수 있어서 그런 걸까요?

[yoonseo8167]

영속화 전에 id가 없을 수 있을 것 같아 ?를 붙였는데, non-nullable로 하는게 널체크를 안해도 돼서 더 나을까요?

[Wo-ogie]

그쵸. Null-safe type을 지원하는 게 kotlin의 이점이니까 최대한 활용하면 좋을 거 같아요.
특히 ID는 DB 인입된 후 null로 절대 변하지 않으니까요.
그러면 영속화 전, DB 인입 전에 id를 null이 아닌 다른 값으로 설정해도 되는지가 중요할 것 같은데요.

실제로 SimpleJpaRepository 구현체 코드를 까보면 다음과 같이 되어있어요.

@Override
@Transactional
public <S extends T> S save(S entity) {

	Assert.notNull(entity, "Entity must not be null");

	if (entityInformation.isNew(entity)) {
		entityManager.persist(entity);
		return entity;
	} else {
		return entityManager.merge(entity);
	}
}

Spring Data JPA가 save()를 할 때, isNew() method를 사용해서 신규 entity라면 persist하고, 아니라면 merge만 하는데요.

public boolean isNew(T entity) {

	ID id = getId(entity);
	Class<ID> idType = getIdType();

	if (!idType.isPrimitive()) {
		return id == null;
	}

	if (id instanceof Number) {
		return ((Number) id).longValue() == 0L;
	}

	throw new IllegalArgumentException(String.format("Unsupported primitive id type %s", idType));
}

AbstractEntityInformation.isNew() method 코드를 보면 id가 null인 경우 뿐만 아니라 0인 경우도 신규 entity로 식별하고 있음을 확인할 수 있습니다. (실제 동작 구현체는 조금 다를 수 있음)

그래서 DB 인입 전에 null을 넣지 않고, 0으로 초기화해도 괜찮아요.
다른 기능들 로직도 한 번 보시면 전부 not null type으로 선언하고 0으로 초기화하게끔 되어 있을 거에요.

추후 entity가 persist 되면, JPA에 의해 0L로 설정됐던 값이 PK로 바뀝니다.

[yoonseo8167]

아하 그렇군요 확인했습니다! not null로 하고 0으로 초기화해놓겠습니다.

[Wo-ogie]

id 필드 not null type으로 바꿔주세요.

[Wo-ogie]

Commit message에 issue 번호 붙여주세요

[Wo-ogie]

DB schema에 변경사항 생기면 schema.sql에 반영해주시면 됩니다.
요건 온보딩때 말씀을 안드렸었네요

[Wo-ogie]

연관 이슈 #163이 아니라 #164인 것 같아요.
이슈에 asignee, Issue type 할당해 주시고, 내용 적당히 채워주세요. 조만간 template 추가할게요.

PR에도 asignees 할당해주세요

그리고 PR 본문에 Closed #164 달아주시면 이슈랑 PR 연결되고, PR merge 되면서 이슈 자동으로 닫히게끔 할 수 있습니다. 참고해주세요!

[Wo-ogie]

이거 혹시 사용하는 class인가요?
아니라면, 삭제해주세요

[Wo-ogie]

TermItem을 Term으로 변환하는 건 TermMapper를 만들어서 MapStruct를 사용해보면 어떨까요?

[Wo-ogie]

Class name에 약관(term)이라는 내용이 들어가면 어떨까 싶은데, 어떤가요?

[Wo-ogie]

불필요한 class는 삭제해주세요. photographer.Request, user.Request에 있는 AgreementRequestItem와 겹치는 거 같아요.
정작 해당 file에서는 추가 구현한 class를 안쓰고 term.Request를 참조하고 있네요.

[Wo-ogie]

다른 jpa entity class를 보면 대부분의 필드 선언은 class 내부에서 하고 있는데요.
필드 선언이 일부는 class 내부에서 하고, 일부는 parameter 선언부에서 섞어서 하면 필드가 많아졌을 때, 파악하기 조금 어려운 것 같아서 이렇게 하고 있습니다. var 필드를 선언하거나 val을 var로 변경하려면 (settter, getter 설정을 위해) 어차피 class 내부로 옮겨야 하기도 하구요.
혹시 어떻게 생각하실까요?

[Wo-ogie]

Field name termJpaRepository로 바꿔주세요.
Entity와 관련된 추가적인 entity에 대한 repository가 정의될 수 있어서, 구체적으로 적는 게 좋은 거 같습니다.

PhotographerCoreRepository 참고