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

대소문자를 구분하지 않는 Attributes #75

Open
wants to merge 15 commits into
base: freddie-noel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 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
38 changes: 28 additions & 10 deletions src/main/java/webserver/http/attribute/Attributes.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,54 @@ public static Attributes from(Map<String, String> attributes) {
}

public static Attributes from(String headerText) {
Map<String, String> attributes = new LinkedHashMap<>();
Attributes attributes = new Attributes();

String[] splittedHeaderTexts = headerText.split(Const.CRLF);
for (String splittedHeaderText : splittedHeaderTexts) {
HttpRequestUtils.Pair pair = HttpRequestUtils.parseHeader(splittedHeaderText);

if (pair != null) {
attributes.put(pair.getKey(), pair.getValue());
attributes.add(pair.getKey(), pair.getValue());
Dae-Hwa marked this conversation as resolved.
Show resolved Hide resolved
}
}

return from(attributes);
return attributes;
}

public Attributes add(String key, String value) {
attributes.put(key, value);
public Attributes add(String targetKey, String value) {
for (String currentKey : attributes.keySet()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍👍👍👍

if (currentKey.equalsIgnoreCase(targetKey)) {
return this;
}
}

attributes.put(targetKey, value);
return this;
}

public Attributes addAll(Map<String, String> attributes) {
this.attributes.putAll(attributes);
for (String currentKey : attributes.keySet()) {
add(currentKey, attributes.get(currentKey));
}

return this;
}

public String get(String key) {
return attributes.get(key);
public String get(String targetKey) {
for (String currentKey : attributes.keySet()) {
if (currentKey.equalsIgnoreCase(targetKey)) {
return attributes.get(currentKey);
}
}
Dae-Hwa marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException("일치하는 키가 없습니다.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

key에 해당하는 value가 null이라면 제대로 동작하지 않을 것 같아요
이런 경우를 막지 않아도 괜찮을까요?
그리고 예외를 발생시키면서까지 get을 사용해야할 경우가 있을까요?
getOrDefault로는 안되는 경우가 있는건지 궁금합니다

Copy link
Owner Author

Choose a reason for hiding this comment

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

key: "null", 
value: null

인 상태에서 key값으로 "null"을 꺼내면 null을 반환 하는데
제대로 동작하지 않는다 게 어떤 부분인지 잘 모르겠습니다.

Copy link
Owner Author

@sanhee sanhee Dec 22, 2021

Choose a reason for hiding this comment

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

f61aa7f 에서 try-catch를 제거하는 방식으로 수정했습니다.

혹시 생각하신 방식이랑 다르다면 말씀해주세요!

}

public String getOrDefault(String key, String defaultValue) {
return attributes.getOrDefault(key, defaultValue);
public String getOrDefault(String targetKey, String defaultValue) {
try {
return get(targetKey);
} catch (IllegalArgumentException e) {
return defaultValue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

try-catch를 안 쓰는 방법도 있지 않을까요??

}

public String toHeaderText() {
Expand Down
16 changes: 10 additions & 6 deletions src/main/java/webserver/http/startline/RequestLine.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
package webserver.http.startline;

import webserver.http.attribute.Attributes;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.*;
import java.util.List;
import java.util.Objects;
import java.util.StringJoiner;

public class RequestLine extends StartLine {

private static final String METHOD_KEY = "method";
private static final String PATH_KEY = "path";

public RequestLine(Map<String, String> statusLineAttributes) {
public RequestLine(Attributes statusLineAttributes) {
super(statusLineAttributes);
}

public static RequestLine from(List<String> statusLine) {
Map<String, String> statusLineAttributes = new HashMap<>();
Attributes statusLineAttributes = new Attributes();

statusLineAttributes.put(METHOD_KEY, statusLine.get(0));
statusLineAttributes.put(PATH_KEY, statusLine.get(1));
statusLineAttributes.put(PROTOCOL_VERSION_KEY, statusLine.get(2));
statusLineAttributes.add(METHOD_KEY, statusLine.get(0));
statusLineAttributes.add(PATH_KEY, statusLine.get(1));
statusLineAttributes.add(PROTOCOL_VERSION_KEY, statusLine.get(2));

return new RequestLine(statusLineAttributes);
}
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/webserver/http/startline/StartLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@

import webserver.http.attribute.Attributes;

import java.util.Map;

public abstract class StartLine {
protected static final String PROTOCOL_VERSION_KEY = "protocolVersion";

private Attributes attributes;

public StartLine(Map<String, String> attributes) {
this.attributes = Attributes.from(attributes);
public StartLine(Attributes attributes) {
this.attributes = attributes;
}

public String getProtocol() {
Expand Down
15 changes: 8 additions & 7 deletions src/main/java/webserver/http/startline/StatusLine.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
package webserver.http.startline;

import java.util.HashMap;
import webserver.http.attribute.Attributes;

import java.util.List;
import java.util.Map;
import java.util.StringJoiner;

public class StatusLine extends StartLine {
private static final String STATUS_CODE_KEY = "statusCode";
private static final String STATUS_TEXT_KEY = "statusText";

public StatusLine(Map<String, String> statusLineAttributes) {

public StatusLine(Attributes statusLineAttributes) {
super(statusLineAttributes);
}

public static StatusLine from(List<String> statusLine) {
Map<String, String> statusLineAttributes = new HashMap<>();
Attributes statusLineAttributes = new Attributes();

statusLineAttributes.put(PROTOCOL_VERSION_KEY, statusLine.get(0));
statusLineAttributes.put(STATUS_CODE_KEY, statusLine.get(1));
statusLineAttributes.put(STATUS_TEXT_KEY, statusLine.get(2));
statusLineAttributes.add(PROTOCOL_VERSION_KEY, statusLine.get(0));
statusLineAttributes.add(STATUS_CODE_KEY, statusLine.get(1));
statusLineAttributes.add(STATUS_TEXT_KEY, statusLine.get(2));

return new StatusLine(statusLineAttributes);
}
Expand Down
67 changes: 62 additions & 5 deletions src/test/java/webserver/http/attribute/AttributesTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package webserver.http.attribute;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand All @@ -11,6 +12,7 @@
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;

class AttributesTest {

Expand Down Expand Up @@ -39,14 +41,42 @@ void fromHeaderText() {
assertThat(actualAttributes).isEqualTo(expectedAttributes);
}

@Test
void add() {

@ParameterizedTest
@DisplayName("add 테스트: key의 대소문자 관계 없이 Attributes를 꺼내올 수 있음")
@MethodSource("add")
void add(String expectedValue, String actualValue) {
assertThat(actualValue).isEqualTo(expectedValue);
}

static Stream<Arguments> add() {
Attributes attributes = new Attributes();
attributes.add("key", "value");

return Stream.of(
Arguments.of(
"value",
attributes.get("KEY")
),
Arguments.of(
"value",
attributes.get("key")
),
Arguments.of(
"value",
attributes.get("kEy")
)
);
}

@Test
void addAll() {
@ParameterizedTest
@DisplayName("addAll 테스트: key의 대소문자 관계 없이 Attributes를 꺼내올 수 있음")
@MethodSource("addAll")
void addAll(String expectedValue, String actualValue) {
assertThat(actualValue).isEqualTo(expectedValue);
}

static Stream<Arguments> addAll() {
Attributes attributes = new Attributes();
Map<String, String> attributeMap = new LinkedHashMap<>();
attributeMap.put("key", "value");
Expand All @@ -55,7 +85,20 @@ void addAll() {
Attributes expectedAttributes = new Attributes();
expectedAttributes.add("key", "value");

assertThat(attributes).isEqualTo(expectedAttributes);
return Stream.of(
Arguments.of(
expectedAttributes.get("key"),
attributes.get("KEY")
),
Arguments.of(
expectedAttributes.get("KEY"),
attributes.get("kEY")
),
Arguments.of(
expectedAttributes.get("kEy"),
attributes.get("kEy")
)
);
}

@Test
Expand All @@ -66,13 +109,27 @@ void get() {
assertThat(attributes.get("key")).isEqualTo("value");
}

@Test
@DisplayName("존재하지 않는 키를 조회하면 예외발생")
void getValueWithEmptyKey() {
Attributes attributes = new Attributes();
assertThatIllegalArgumentException().isThrownBy(() -> attributes.get("emptyKey"));
}

@ParameterizedTest
@MethodSource("getOrDefault")
void getOrDefault(Attributes attributes, String defaultValue, String key, String expectedValue) {
String actualValue = attributes.getOrDefault(key, defaultValue);
assertThat(actualValue).isEqualTo(expectedValue);
}

@Test
@DisplayName("존재하지 않는 키를 조회하면 주어진 기본값으로 대체")
void getDefaultValueWithEmptyKey() {
Attributes attributes = new Attributes();
assertThat(attributes.getOrDefault("emptyKey", "happy")).isEqualTo("happy");
}

static Stream<Arguments> getOrDefault() {
return Stream.of(
Arguments.of(
Expand Down
20 changes: 10 additions & 10 deletions src/test/java/webserver/http/header/RequestHeaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,11 @@ static Stream<Arguments> getRequestLineAttributes() {
"Cookie: Idea-1c77831=5ced54c8-cabd-4355-ae5a-97b17f9d7443" + Const.CRLF +
Const.CRLF,
new RequestLine(
new HashMap<String, String>() {{
Attributes.from(new HashMap<String, String>() {{
put("method", "GET");
put("path", "/");
put("protocolVersion", "HTTP/1.1");
}}
}})
)
)
);
Expand Down Expand Up @@ -217,21 +217,21 @@ static Stream<Arguments> getPath() {
Arguments.of(
"쿼리스트링이 없는 GET 메세지",
new RequestLine(
new HashMap() {{
put("path", "/user/create");
put("method", "GET");
put("protocolVersion", "HTTP/1.1");
new Attributes() {{
add("path", "/user/create");
add("method", "GET");
add("protocolVersion", "HTTP/1.1");
}}
),
"/user/create"
),
Arguments.of(
"쿼리스트링이 포함된 GET 메세지 path를 출력할때도 쿼리스트링이 포함되지 않아야 함",
new RequestLine(
new HashMap() {{
put("path", "/user/create?userId=javajigi&password=password&name=%EB%B0%95%EC%9E%AC%EC%84%B1&email=javajigi%40slipp.net");
put("method", "GET");
put("protocolVersion", "HTTP/1.1");
new Attributes() {{
add("path", "/user/create?userId=javajigi&password=password&name=%EB%B0%95%EC%9E%AC%EC%84%B1&email=javajigi%40slipp.net");
add("method", "GET");
add("protocolVersion", "HTTP/1.1");
}}
),
"/user/create"
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/webserver/http/header/ResponseHeaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ static Stream<Arguments> getStatusLineAttributes() {
"Content-Length: " + "Hello World".getBytes().length + Const.CRLF +
Const.CRLF,
new StatusLine(
new HashMap() {{
Attributes.from(new HashMap<String, String>() {{
put("protocolVersion", "HTTP/1.1");
put("statusText", "OK");
put("statusCode", "200");
}}
}})
)
)
);
Expand Down
Loading