Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
adutra committed Dec 16, 2024
1 parent 56ff7b2 commit ca6eb1e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 18 deletions.
11 changes: 6 additions & 5 deletions core/src/main/java/org/apache/iceberg/rest/HTTPHeaders.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.iceberg.rest;

import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.immutables.value.Value;
Expand All @@ -40,13 +41,13 @@ public interface HTTPHeaders {
HTTPHeaders EMPTY = of();

/** Returns all the header entries in this group. */
List<HTTPHeader> entries();
Set<HTTPHeader> entries();

/** Returns all the entries in this group for the given name (case-insensitive). */
default List<HTTPHeader> entries(String name) {
default Set<HTTPHeader> entries(String name) {
return entries().stream()
.filter(header -> header.name().equalsIgnoreCase(name))
.collect(Collectors.toList());
.collect(Collectors.toSet());
}

/** Returns whether this group contains an entry with the given name (case-insensitive). */
Expand All @@ -59,7 +60,7 @@ default boolean contains(String name) {
* Returns a new instance with the added header, or the current instance if the header is already
* present.
*/
default HTTPHeaders withHeaderIfAbsent(HTTPHeader header) {
default HTTPHeaders putIfAbsent(HTTPHeader header) {
Preconditions.checkNotNull(header, "header");
return contains(header.name())
? this
Expand All @@ -71,7 +72,7 @@ default HTTPHeaders withHeaderIfAbsent(HTTPHeader header) {
* Returns a new instance with the added headers, or the current instance if all headers are
* already present.
*/
default HTTPHeaders withHeaderIfAbsent(HTTPHeaders headers) {
default HTTPHeaders putIfAbsent(HTTPHeaders headers) {
Preconditions.checkNotNull(headers, "headers");
List<HTTPHeader> newHeaders =
headers.entries().stream().filter(e -> !contains(e.name())).collect(Collectors.toList());
Expand Down
29 changes: 16 additions & 13 deletions core/src/test/java/org/apache/iceberg/rest/TestHTTPHeaders.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ class TestHTTPHeaders {
@Test
void entries() {
assertThat(headers.entries("header1"))
.containsExactly(HTTPHeader.of("header1", "value1a"), HTTPHeader.of("HEADER1", "value1b"));
.containsExactlyInAnyOrder(
HTTPHeader.of("header1", "value1a"), HTTPHeader.of("HEADER1", "value1b"));
assertThat(headers.entries("HEADER1"))
.containsExactly(HTTPHeader.of("header1", "value1a"), HTTPHeader.of("HEADER1", "value1b"));
assertThat(headers.entries("header2")).containsExactly(HTTPHeader.of("header2", "value2"));
assertThat(headers.entries("HEADER2")).containsExactly(HTTPHeader.of("header2", "value2"));
.containsExactlyInAnyOrder(
HTTPHeader.of("header1", "value1a"), HTTPHeader.of("HEADER1", "value1b"));
assertThat(headers.entries("header2"))
.containsExactlyInAnyOrder(HTTPHeader.of("header2", "value2"));
assertThat(headers.entries("HEADER2"))
.containsExactlyInAnyOrder(HTTPHeader.of("header2", "value2"));
assertThat(headers.entries("header3")).isEmpty();
assertThat(headers.entries("HEADER3")).isEmpty();
assertThat(headers.entries(null)).isEmpty();
Expand All @@ -57,31 +61,30 @@ void contains() {
}

@Test
void withHeaderIfAbsentHTTPHeader() {
HTTPHeaders actual = headers.withHeaderIfAbsent(HTTPHeader.of("Header1", "value1c"));
void putIfAbsentHTTPHeader() {
HTTPHeaders actual = headers.putIfAbsent(HTTPHeader.of("Header1", "value1c"));
assertThat(actual).isSameAs(headers);

actual = headers.withHeaderIfAbsent(HTTPHeader.of("header3", "value3"));
actual = headers.putIfAbsent(HTTPHeader.of("header3", "value3"));
assertThat(actual.entries())
.containsExactly(
HTTPHeader.of("header1", "value1a"),
HTTPHeader.of("HEADER1", "value1b"),
HTTPHeader.of("header2", "value2"),
HTTPHeader.of("header3", "value3"));

assertThatThrownBy(() -> headers.withHeaderIfAbsent((HTTPHeader) null))
assertThatThrownBy(() -> headers.putIfAbsent((HTTPHeader) null))
.isInstanceOf(NullPointerException.class)
.hasMessage("header");
}

@Test
void withHeaderIfAbsentHTTPHeaders() {
HTTPHeaders actual =
headers.withHeaderIfAbsent(HTTPHeaders.of(HTTPHeader.of("Header1", "value1c")));
void putIfAbsentHTTPHeaders() {
HTTPHeaders actual = headers.putIfAbsent(HTTPHeaders.of(HTTPHeader.of("Header1", "value1c")));
assertThat(actual).isSameAs(headers);

actual =
headers.withHeaderIfAbsent(
headers.putIfAbsent(
ImmutableHTTPHeaders.builder()
.addEntry(HTTPHeader.of("Header1", "value1c"))
.addEntry(HTTPHeader.of("header3", "value3"))
Expand All @@ -96,7 +99,7 @@ void withHeaderIfAbsentHTTPHeaders() {
HTTPHeader.of("header3", "value3"))
.build());

assertThatThrownBy(() -> headers.withHeaderIfAbsent((HTTPHeaders) null))
assertThatThrownBy(() -> headers.putIfAbsent((HTTPHeaders) null))
.isInstanceOf(NullPointerException.class)
.hasMessage("headers");
}
Expand Down

0 comments on commit ca6eb1e

Please sign in to comment.