Skip to content
This repository was archived by the owner on Dec 18, 2022. It is now read-only.

Origin/feature/49 multipart#125

Open
pocketmun1216 wants to merge 46 commits intoztkmkoo:developfrom
pocketmun1216:origin/feature/49_multipart
Open

Origin/feature/49 multipart#125
pocketmun1216 wants to merge 46 commits intoztkmkoo:developfrom
pocketmun1216:origin/feature/49_multipart

Conversation

@pocketmun1216
Copy link
Copy Markdown

multipart 처리할 수 있게 하였습니다.

리뷰하실 때

  1. 테스팅 코드
    DssRestServerTest <- 테스팅 코드 추가

  2. Header 파싱 방법 제안
    DssRestRequest <- header의 charset과 boundary 파싱하는 코드를 추가하였습니다.

  3. Body 파싱 방법 제안
    DssRestActorFormDataService <- body 부분의 boundary 기준으로 파싱하는 코드를 추가하였습니다.

  4. DssRestActorService
    기존 DssRestServiceResponse handling(String content) -> DssRestServiceResponse handling(DssRestServiceActorCommandRequest commandRequest)
    로 변경되었습니다. 인터페이스가 변경됨에 따라 인터페이스를 구현하는 하위 클래스 또한 모두 변경되었습니다.
    변경 한 이유는
    DssRestActorFormDataService <- 해당 클래스에서 content 파싱시 formdata형식은 boundary로 파싱해야 하는데
    기존에 content만 넘길 경우 boundary를 넘겨받기가 어렵기 때문에 헤더정보를 같이 넘겨주는 DssRestServiceActorCommandRequest 로 변경하였습니다.

  5. 실행 예제
    포스트맨으로 테스팅 해보고 싶은 분들은 MultiPart 클래스 <- 참고하시면 됩니다.

@ztkmkoo ztkmkoo self-requested a review September 3, 2020 23:32
@ztkmkoo ztkmkoo added feature New feature korean issues in korean service service job labels Sep 3, 2020
@ztkmkoo ztkmkoo added this to the Contributhon milestone Sep 3, 2020
@ztkmkoo ztkmkoo linked an issue Sep 3, 2020 that may be closed by this pull request
@ztkmkoo
Copy link
Copy Markdown
Owner

ztkmkoo commented Sep 3, 2020

PR 감사합니다.

우선 conflict 해결 및 IDE 파일 .iml은 제거 해주시면 감사하겠습니다.

Comment on lines +15 to +22
<dependencies>
<dependency>
<groupId>io.github.ztkmkoo</groupId>
<artifactId>dss-server</artifactId>
<version>0.4.4</version>
<scope>compile</scope>
</dependency>
</dependencies>
Copy link
Copy Markdown
Collaborator

@RulLu16 RulLu16 Sep 4, 2020

Choose a reason for hiding this comment

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

이미 이전 dss-example/pom.xml에서 dss-server에 대한 dependency가 있기 때문에 굳이 바꿀 이유가 없어보입니다.

RulLu16, Alencion, ztkmkoo 님 리뷰내용을 반영하였습니다. 감사합니다.
@ztkmkoo
Copy link
Copy Markdown
Owner

ztkmkoo commented Sep 4, 2020

PR 감사합니다.

우선 conflict 해결 및 IDE 파일 .iml은 제거 해주시면 감사하겠습니다.

@MunSeongUk
44f7104 요기 커밋에서 리뷰 반영이라고 되어 있는데, conflict가 해결 안된것 같은데 이건 아직 수정 안하신거죠?

@coveralls
Copy link
Copy Markdown

coveralls commented Sep 10, 2020

Coverage Status

Coverage decreased (-3.2%) to 82.196% when pulling 67aad88 on MunSeongUk:origin/feature/49_multipart into 5106899 on ztkmkoo:develop.


@Test
void constructor3() {
public void constructor3() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems to be better to remove access modifier public.

Copy link
Copy Markdown
Collaborator

@Doyuni Doyuni left a comment

Choose a reason for hiding this comment

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

Hi, good job about handling multipart.

But It needs to be fetched from upstream and seems to be better to remove public access modifier in test code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New feature korean issues in korean service service job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]Rest request with content-type form-data not working

6 participants