Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public NotificationMessage process(UsageNotificationOutboxRow item) {
return new NotificationMessage(
UUID.randomUUID(),
item.id(),
101L,
2L,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

templateGroupId로 사용되는 2L은 의미를 알기 어려운 매직 넘버입니다. 코드의 가독성과 유지보수성을 높이기 위해, 이 값의 의미를 명확히 나타내는 상수로 정의하는 것을 고려해 보세요. 예를 들어, NotificationTemplateConstants와 같은 별도의 상수 관리 클래스에 USAGE_NOTIFICATION_TEMPLATE_GROUP_ID와 같이 정의하여 사용할 수 있습니다.

Map.of(
"subId", item.subId(),
"phoneNumber", item.phoneNumber(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import org.springframework.transaction.PlatformTransactionManager;

import com.template.worker.global.listener.JobResultListener;
import com.template.worker.jobs.invoicesend.model.InvoiceAggregateRecord;
import com.template.worker.jobs.invoicesend.model.InvoiceNotificationEvent;
import com.template.worker.jobs.invoicesend.model.InvoiceSendRecord;
import com.template.worker.jobs.invoicesend.processor.InvoiceSendProcessor;
import com.template.worker.jobs.invoicesend.writer.InvoiceSendWriter;
Expand Down Expand Up @@ -45,7 +45,7 @@ public Job invoiceSendJob(Step invoiceSendStep) {
@Bean
public Step invoiceSendStep(JdbcCursorItemReader<InvoiceSendRecord> invoiceJoinReader) {
return new StepBuilder("invoiceSendStep", jobRepository)
.<InvoiceSendRecord, InvoiceAggregateRecord>chunk(chunkSize, transactionManager)
.<InvoiceSendRecord, InvoiceNotificationEvent>chunk(chunkSize, transactionManager)
.reader(invoiceJoinReader)
.processor(invoiceSendProcessor)
.writer(invoiceSendWriter)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.template.worker.jobs.invoicesend.model;

import java.util.List;
import java.util.UUID;

public record InvoiceNotificationEvent(
UUID eventId,
Long templateGroupId,
SubscriptionInfo subscriptionInfo,
Variables variables) {

public record SubscriptionInfo(Long subId, String phoneNumber, String email) {}

public record Variables(
String invoiceNumber,
String customerName,
String phoneNumber,
String planName,
String billingDate,
String dueDate,
Integer totalAmount,
String paymentStatus,
List<Item> items) {}

public record Item(String name, String detail, Integer amount) {}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,34 @@
package com.template.worker.jobs.invoicesend.processor;

import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

import org.springframework.batch.core.configuration.annotation.StepScope;
import org.springframework.batch.item.ItemProcessor;
import org.springframework.stereotype.Component;

import com.template.worker.jobs.invoicesend.model.InvoiceAggregateRecord;
import com.template.worker.jobs.invoicesend.model.InvoiceItemRecord;
import com.template.worker.jobs.invoicesend.model.InvoiceNotificationEvent;
import com.template.worker.jobs.invoicesend.model.InvoiceSendRecord;

@Component
@StepScope
public class InvoiceSendProcessor
implements ItemProcessor<InvoiceSendRecord, InvoiceAggregateRecord> {
implements ItemProcessor<InvoiceSendRecord, InvoiceNotificationEvent> {

private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd");

private Long currentInvId;
private InvoiceSendRecord currentInvoice;
private List<InvoiceItemRecord> currentItems;

@Override
public InvoiceAggregateRecord process(InvoiceSendRecord row) {
public InvoiceNotificationEvent process(InvoiceSendRecord row) {

if (currentInvId == null) {
startNewInvoice(row);
start(row);
return null;
}

Expand All @@ -33,14 +37,12 @@ public InvoiceAggregateRecord process(InvoiceSendRecord row) {
return null;
}

InvoiceAggregateRecord finished = buildAggregate();

startNewInvoice(row);

InvoiceNotificationEvent finished = buildEvent();
start(row);
return finished;
}

private void startNewInvoice(InvoiceSendRecord row) {
private void start(InvoiceSendRecord row) {
this.currentInvId = row.invId();
this.currentInvoice = row;
this.currentItems = new ArrayList<>(4);
Expand All @@ -56,26 +58,58 @@ private void addItem(InvoiceSendRecord row) {
row.itemValue().intValue()));
}

public InvoiceAggregateRecord flushLast() {
public InvoiceNotificationEvent flushLast() {
if (currentInvId == null) {
return null;
}
return buildAggregate();
return buildEvent();
}

private InvoiceNotificationEvent buildEvent() {

String planName =
currentItems.stream()
.filter(i -> "PLAN".equals(i.invoiceType()))
.map(InvoiceItemRecord::invoiceName)
.findFirst()
.orElse(null);

List<InvoiceNotificationEvent.Item> items =
currentItems.stream()
.map(
i ->
new InvoiceNotificationEvent.Item(
mapItemName(i.invoiceType()),
i.invoiceName(),
i.value()))
.toList();

return new InvoiceNotificationEvent(
UUID.randomUUID(),
1L, // templateGroupId 고정
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

templateGroupId로 사용되는 1L은 매직 넘버입니다. 이 값의 의미를 명확히 나타내는 상수로 정의하여 가독성과 유지보수성을 향상시키는 것이 좋습니다. 예를 들어, INVOICE_NOTIFICATION_TEMPLATE_GROUP_ID와 같은 이름의 상수를 사용할 수 있습니다. NotificationSendProcessor에서 발견된 매직 넘버와 함께 NotificationTemplateConstants와 같은 클래스에서 상수를 중앙 관리하면 더욱 체계적인 코드가 될 것입니다.

new InvoiceNotificationEvent.SubscriptionInfo(
currentInvoice.subId(),
currentInvoice.phone_enc(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

사용하시는 currentInvoice 객체의 phone_enc()email_enc() 메소드 호출은 InvoiceSendRecord의 필드명이 Java 네이밍 컨벤션(camelCase)을 따르지 않고 있음을 보여줍니다. 코드의 일관성과 가독성을 위해 InvoiceSendRecordphone_enc, email_enc 필드를 phoneEnc, emailEnc와 같이 camelCase로 리팩토링하는 것을 고려해 보세요. 이 변경은 InvoiceSendRecord를 생성하는 부분(예: DB 쿼리)에도 영향을 줄 수 있으므로 관련 코드도 함께 수정해야 합니다.

currentInvoice.email_enc()),
new InvoiceNotificationEvent.Variables(
currentInvoice.invNo().toString(),
currentInvoice.name(),
currentInvoice.phone_enc(),
planName,
currentInvoice.createdAt().format(DATE_FORMAT),
currentInvoice.dueDate().format(DATE_FORMAT),
currentInvoice.totalPrice(),
"PAID",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

paymentStatus에 하드코딩된 문자열 "PAID"가 사용되었습니다. 이러한 값은 오타에 취약하고, 나중에 상태가 추가/변경될 때 관리가 어렵습니다. PaymentStatus와 같은 enum을 정의하거나, 최소한 상수로 분리하여 관리하는 것을 권장합니다.

개선 예시:

public enum PaymentStatus {
    PAID, UNPAID, FAILED;
}

// ...
new InvoiceNotificationEvent.Variables(
    // ...
    PaymentStatus.PAID.name(),
    items
);

items));
}

private InvoiceAggregateRecord buildAggregate() {
return new InvoiceAggregateRecord(
2L,
currentInvoice.invNo(),
currentInvoice.invId(),
currentInvoice.subId(),
currentInvoice.invMonth(),
currentInvoice.phone_enc(),
currentInvoice.email_enc(),
currentInvoice.name(),
currentInvoice.totalPrice(),
currentInvoice.createdAt(),
currentInvoice.dueDate(),
List.copyOf(currentItems));
private String mapItemName(String type) {
return switch (type) {
case "PLAN" -> "기본 요금";
case "VAS" -> "부가서비스";
case "DISCOUNT" -> "할인";
case "MICRO" -> "소액 결제";
default -> "기타";
};
}
Comment on lines +106 to 114
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

mapItemName 메소드와 buildEvent 메소드 내에서 "PLAN", "VAS" 등 invoiceType을 나타내는 문자열이 하드코딩되어 사용되고 있습니다. (e.g., L72, L108-L111)

이러한 방식은 오타에 취약하며, 새로운 타입이 추가될 때 여러 곳을 수정해야 하는 불편함이 있습니다.

InvoiceItemType과 같은 enum을 정의하여 타입 안정성을 높이고 유지보수성을 개선하는 것을 강력히 권장합니다. enum을 사용하면 관련 로직을 한 곳에서 관리할 수 있습니다.

개선 예시:

public enum InvoiceItemType {
    PLAN("기본 요금"),
    VAS("부가서비스"),
    DISCOUNT("할인"),
    MICRO("소액 결제"),
    ETC("기타");

    private final String koreanName;

    InvoiceItemType(String koreanName) {
        this.koreanName = koreanName;
    }

    public String getKoreanName() {
        return koreanName;
    }

    public static InvoiceItemType fromString(String text) {
        if (text != null) {
            for (InvoiceItemType b : InvoiceItemType.values()) {
                if (text.equalsIgnoreCase(b.name())) {
                    return b;
                }
            }
        }
        return ETC;
    }
}

// InvoiceSendProcessor.java

private InvoiceNotificationEvent buildEvent() {
    String planName = currentItems.stream()
            .filter(i -> InvoiceItemType.PLAN.name().equals(i.invoiceType())) // enum 사용
            .map(InvoiceItemRecord::invoiceName)
            .findFirst()
            .orElse(null);
    // ...
}

private String mapItemName(String type) {
    return InvoiceItemType.fromString(type).getKoreanName();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.springframework.stereotype.Component;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.template.worker.jobs.invoicesend.model.InvoiceAggregateRecord;
import com.template.worker.jobs.invoicesend.model.InvoiceNotificationEvent;
import com.template.worker.jobs.invoicesend.processor.InvoiceSendProcessor;

import lombok.RequiredArgsConstructor;
Expand All @@ -16,20 +16,20 @@
@Slf4j
@Component
@RequiredArgsConstructor
public class InvoiceSendWriter implements ItemWriter<InvoiceAggregateRecord>, ItemStream {
public class InvoiceSendWriter implements ItemWriter<InvoiceNotificationEvent>, ItemStream {

private static final String TOPIC = "invoice-noti";

private final KafkaTemplate<String, String> kafkaTemplate;
private final ObjectMapper objectMapper;
private final InvoiceSendProcessor processor;

private InvoiceAggregateRecord lastBuffered;
private InvoiceNotificationEvent lastBuffered;

@Override
public void write(Chunk<? extends InvoiceAggregateRecord> items) throws Exception {
public void write(Chunk<? extends InvoiceNotificationEvent> items) throws Exception {

for (InvoiceAggregateRecord invoice : items) {
for (InvoiceNotificationEvent invoice : items) {
send(invoice);
}

Expand All @@ -53,8 +53,8 @@ public void close() {
}
}

private void send(InvoiceAggregateRecord invoice) throws Exception {
private void send(InvoiceNotificationEvent invoice) throws Exception {
String payload = objectMapper.writeValueAsString(invoice);
kafkaTemplate.send(TOPIC, String.valueOf(invoice.invId()), payload);
kafkaTemplate.send(TOPIC, String.valueOf(invoice.eventId()), payload);
}
}
Loading