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

Feature/shipper2 #4

Merged
merged 7 commits into from
Jan 13, 2025
Merged

Feature/shipper2 #4

merged 7 commits into from
Jan 13, 2025

Conversation

riderkaman
Copy link
Collaborator

No description provided.

public interface CarRepository extends JpaRepository<Car, String> {
import java.util.Optional;

public interface CarRepository {
Copy link
Collaborator

Choose a reason for hiding this comment

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

꼭 인터페이스를 만들어야 하나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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


@BeforeEach
void setUp() throws Exception {
this.truckerService = TruckerService.builder()
Copy link
Collaborator

Choose a reason for hiding this comment

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

@InjectMock @mock


class TruckerServiceTest {

private TruckerService truckerService;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
private TruckerService truckerService;
@InjectMock
private TruckerService truckerService;
@Mock
private CarRepository carReposityr;
...


@Test
void createContract_성공하는_경우() {
TruckerDTO.ContractRequest req = new TruckerDTO.ContractRequest();
Copy link
Collaborator

@f-lab-lyan f-lab-lyan Nov 19, 2024

Choose a reason for hiding this comment

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

Suggested change
TruckerDTO.ContractRequest req = new TruckerDTO.ContractRequest();
// when
when(carRepository.findById(any())).thenReturn(Optional.OfNullable(..));
TruckerDTO.ContractRequest req = new TruckerDTO.ContractRequest();

@Slf4j
@Service
@RequiredArgsConstructor
@Builder
Copy link
Collaborator

Choose a reason for hiding this comment

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

테스트를 위해서 라이브코드를 고치는 것이 좋은 것인가?

@Slf4j
@Service
@RequiredArgsConstructor
@Builder
public class TruckerService {

private final CarRepository carRepository;
Copy link
Collaborator

Choose a reason for hiding this comment

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

서비스에서 다른 리포지토리를 호출 하는 것에 대해서

2. test 는 Mock 사용하도록 수정
3. repository 는 interface 만 사용하도록 수정
Comment on lines +17 to +19
private final AccountService accountService;
private final CarService carService;
private final ContractService contractService;
Copy link
Collaborator

Choose a reason for hiding this comment

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

전 여전히 각각의 서비스들을 컨트롤러에 가져오는 것의 이점을 아직 잘 모르겠어요!! 이 경우에도 특정 URL은 하나의 서비스만 사용하는 것 같기도 하구요. :-)

String contractId = truckerService.createContract(req);
ShipmentDTO.BasicInfo basicInfo = new ShipmentDTO.BasicInfo();
basicInfo.setShipmentStatus("ready");
ResponseEntity<ShipmentDTO.BasicInfo> basicInfoResponseEntity = new ResponseEntity<ShipmentDTO.BasicInfo>(basicInfo, HttpStatus.OK);
Copy link
Collaborator

Choose a reason for hiding this comment

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

코드를 잠깐 봤는데, BasicInfoResponseEntity 자체를 mocking 해볼 수도 있을 것 같네요. :-) 이 얘기는 이따 멘토링 시간에 얘기해보죠. ㅎㅎ

ShipmentDTO.BasicInfo basicInfo = new ShipmentDTO.BasicInfo();
basicInfo.setShipmentStatus("ready");
ResponseEntity<ShipmentDTO.BasicInfo> basicInfoResponseEntity = new ResponseEntity<ShipmentDTO.BasicInfo>(basicInfo, HttpStatus.OK);
Contract contract = Contract.builder().contractId("testId").build();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Contract contract = Contract.builder().contractId("testId").build();
Contract contract = mock(Contract.class);
when(contract.getContractId()).thenReturn("testId");

Comment on lines +52 to +54
ShipmentDTO.BasicInfo basicInfo = new ShipmentDTO.BasicInfo();
basicInfo.setShipmentStatus("ready");
ResponseEntity<ShipmentDTO.BasicInfo> basicInfoResponseEntity = new ResponseEntity<ShipmentDTO.BasicInfo>(basicInfo, HttpStatus.OK);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ShipmentDTO.BasicInfo basicInfo = new ShipmentDTO.BasicInfo();
basicInfo.setShipmentStatus("ready");
ResponseEntity<ShipmentDTO.BasicInfo> basicInfoResponseEntity = new ResponseEntity<ShipmentDTO.BasicInfo>(basicInfo, HttpStatus.OK);
ShipmentDTO.BasicInfo basicInfo = mock(ShipmentDTO.BasicInfo.class);
when(basicInfo.getShipmentStatus()).thenReturn("ready");
ResponseEntity<ShipmentDTO.BasicInfo> basicInfoResponseEntity = mock(...);
when(basicInfoReponseEntity.getBasicInfo()).thenReturn(basicInfo);

Copy link
Collaborator

@f-lab-lyan f-lab-lyan left a comment

Choose a reason for hiding this comment

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

@async를 붙여서 Spring의 도움을 받아서 로직을 변경한 것도 좋은데요. 이왕이면, 직접 CompletableFuture를 써서, 로직 자체를 변경해 보면, 좋을 것 같습니다. 그리고 @async를 붙이면, 실제로는 어떤 식으로 동작하는지에 대해서도 고민해보고 설명할 수 있으면 좋겠습니다. :-)

@@ -16,12 +17,14 @@ public class CarService {

private final CarRepository carRepository;

@Async(value = "taskExecutor")
public String createCar(TruckerDTO.CarRequest req) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public String createCar(TruckerDTO.CarRequest req) {
String car = createCar(...);

Copy link
Collaborator

@f-lab-lyan f-lab-lyan left a comment

Choose a reason for hiding this comment

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

@async 어노테이션에 대해서 좀 더 생각해보면 좋을 것 같습니다.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
3.6% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link
Collaborator

@f-lab-lyan f-lab-lyan left a comment

Choose a reason for hiding this comment

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

고생하셨어요! 전에 간략하게 말씀드렸던대로, protobuf를 하나의 모듈로 통합해서 공유하는 방법도 생각해보셨으면 좋겠습니다. :-)

@riderkaman riderkaman merged commit 7f6280a into main Jan 13, 2025
1 of 2 checks passed
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