Skip to content

Commit b800b6b

Browse files
damianmalczewskidamianmalczewski
committed
Unset extension if null provided
1 parent f85a9cf commit b800b6b

7 files changed

Lines changed: 107 additions & 131 deletions

File tree

‎src/main/java/io/github/problem4j/core/AbstractProblem.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,11 @@ public String getKey() {
386386
*
387387
* @param value new value
388388
* @return the new value
389+
* @deprecated This method exists only to satisfy the {@link Map.Entry} contract inherited by
390+
* {@link Extension} and will be removed in a future major version and the object will be
391+
* truly immutable.
389392
*/
393+
@Deprecated
390394
@Override
391395
public @Nullable Object setValue(@Nullable Object value) {
392396
this.value = value;

‎src/main/java/io/github/problem4j/core/AbstractProblemBuilder.java‎

Lines changed: 8 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.io.Serializable;
2525
import java.net.URI;
2626
import java.util.ArrayList;
27-
import java.util.Collection;
2827
import java.util.HashMap;
2928
import java.util.List;
3029
import java.util.Map;
@@ -170,72 +169,25 @@ public ProblemBuilder instance(@Nullable String instance) {
170169
}
171170

172171
/**
173-
* Adds a single custom extension.
172+
* Adds or removes a single custom extension. If {@code value} is {@code null} and an extension
173+
* with the given {@code name} already exists, it will be removed.
174174
*
175175
* @param name the extension key
176-
* @param value the extension value
176+
* @param value the extension value, or {@code null} to remove
177177
* @return this builder instance for chaining
178178
*/
179179
@Override
180180
public ProblemBuilder extension(@Nullable String name, @Nullable Object value) {
181-
if (name != null && value != null) {
182-
extensions.put(name, value);
183-
}
184-
return this;
185-
}
186-
187-
/**
188-
* Adds multiple custom extensions from a map.
189-
*
190-
* @param extensions map of extension keys and values
191-
* @return this builder instance for chaining
192-
*/
193-
@Override
194-
public ProblemBuilder extensions(@Nullable Map<String, ? extends @Nullable Object> extensions) {
195-
if (extensions != null) {
196-
extensions.forEach(
197-
(key, value) -> {
198-
if (value != null) {
199-
this.extensions.put(key, value);
200-
}
201-
});
202-
}
203-
return this;
204-
}
205-
206-
/**
207-
* Adds multiple custom extensions from varargs of {@link Problem.Extension}.
208-
*
209-
* @param extensions array of extensions
210-
* @return this builder instance for chaining
211-
*/
212-
@Override
213-
public ProblemBuilder extensions(Problem.@Nullable Extension @Nullable ... extensions) {
214-
if (extensions != null) {
215-
for (Problem.@Nullable Extension e : extensions) {
216-
if (e != null && e.getValue() != null) {
217-
this.extensions.put(e.getKey(), e.getValue());
218-
}
181+
if (name != null) {
182+
if (value != null) {
183+
extensions.put(name, value);
184+
} else {
185+
extensions.remove(name);
219186
}
220187
}
221188
return this;
222189
}
223190

224-
/**
225-
* Adds multiple custom extensions from a collection of {@link Problem.Extension}.
226-
*
227-
* @param extensions collection of extensions
228-
* @return this builder instance for chaining
229-
*/
230-
@Override
231-
public ProblemBuilder extensions(
232-
@Nullable Collection<? extends Problem.@Nullable Extension> extensions) {
233-
if (extensions != null && !extensions.isEmpty()) {
234-
extensions(extensions.toArray(new Problem.Extension[0]));
235-
}
236-
return this;
237-
}
238-
239191
/**
240192
* Builds an immutable {@link Problem} instance with the configured properties and extensions.
241193
*

‎src/main/java/io/github/problem4j/core/Problem.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,11 @@ interface Extension extends Map.Entry<String, @Nullable Object> {
264264
*
265265
* @param value new value
266266
* @return the new value
267+
* @deprecated This method exists only to satisfy the {@link Map.Entry} contract inherited by
268+
* {@link Extension} and will be removed in a future major version and the object will be
269+
* truly immutable.
267270
*/
271+
@Deprecated
268272
@Override
269273
@Nullable Object setValue(@Nullable Object value);
270274
}

‎src/main/java/io/github/problem4j/core/ProblemBuilder.java‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,11 @@ public interface ProblemBuilder {
101101
ProblemBuilder instance(@Nullable String instance);
102102

103103
/**
104-
* Adds a single custom extension.
104+
* Adds or removes a single custom extension. If {@code value} is {@code null} and an extension
105+
* with the given {@code name} already exists, it will be removed.
105106
*
106107
* @param name the extension key
107-
* @param value the extension value
108+
* @param value the extension value, or {@code null} to remove
108109
* @return this builder instance for chaining
109110
*/
110111
ProblemBuilder extension(@Nullable String name, @Nullable Object value);
@@ -115,7 +116,12 @@ public interface ProblemBuilder {
115116
* @param extensions map of extension keys and values
116117
* @return this builder instance for chaining
117118
*/
118-
ProblemBuilder extensions(@Nullable Map<String, ? extends @Nullable Object> extensions);
119+
default ProblemBuilder extensions(@Nullable Map<String, ? extends @Nullable Object> extensions) {
120+
if (extensions != null) {
121+
extensions.forEach(this::extension);
122+
}
123+
return this;
124+
}
119125

120126
/**
121127
* Adds single custom extension from {@link Problem.Extension}.
@@ -136,15 +142,30 @@ default ProblemBuilder extension(Problem.@Nullable Extension extension) {
136142
* @param extensions array of extensions
137143
* @return this builder instance for chaining
138144
*/
139-
ProblemBuilder extensions(Problem.@Nullable Extension @Nullable ... extensions);
145+
default ProblemBuilder extensions(Problem.@Nullable Extension @Nullable ... extensions) {
146+
if (extensions != null) {
147+
for (Problem.@Nullable Extension e : extensions) {
148+
if (e != null) {
149+
extension(e);
150+
}
151+
}
152+
}
153+
return this;
154+
}
140155

141156
/**
142157
* Adds multiple custom extensions from a collection of {@link Problem.Extension}.
143158
*
144159
* @param extensions collection of extensions
145160
* @return this builder instance for chaining
146161
*/
147-
ProblemBuilder extensions(@Nullable Collection<? extends Problem.@Nullable Extension> extensions);
162+
default ProblemBuilder extensions(
163+
@Nullable Collection<? extends Problem.@Nullable Extension> extensions) {
164+
if (extensions != null) {
165+
extensions.forEach(this::extension);
166+
}
167+
return this;
168+
}
148169

149170
/**
150171
* Builds an immutable {@link Problem} instance with the configured properties and extensions.
@@ -175,7 +196,7 @@ default ProblemBuilder extension(Problem.@Nullable Extension extension) {
175196
*
176197
* @param extensions map of extension keys and values
177198
* @return this builder instance for chaining
178-
* @deprecated use {@link #extensions(Map)} instead
199+
* @deprecated Use {@link #extensions(Map)} instead.
179200
*/
180201
@Deprecated
181202
default ProblemBuilder extension(@Nullable Map<String, ? extends @Nullable Object> extensions) {
@@ -190,7 +211,7 @@ default ProblemBuilder extension(@Nullable Map<String, ? extends @Nullable Objec
190211
*
191212
* @param extensions array of extensions
192213
* @return this builder instance for chaining
193-
* @deprecated use {@link #extensions(Problem.Extension...)} instead
214+
* @deprecated Use {@link #extensions(Problem.Extension...)} instead.
194215
*/
195216
@Deprecated
196217
default ProblemBuilder extension(Problem.@Nullable Extension @Nullable ... extensions) {
@@ -205,7 +226,7 @@ default ProblemBuilder extension(Problem.@Nullable Extension @Nullable ... exten
205226
*
206227
* @param extensions collection of extensions
207228
* @return this builder instance for chaining
208-
* @deprecated use {@link #extensions(Collection)} instead
229+
* @deprecated Use {@link #extensions(Collection)} instead.
209230
*/
210231
@Deprecated
211232
default ProblemBuilder extension(

‎src/test/java/io/github/problem4j/core/MapUtils.java‎

Lines changed: 0 additions & 48 deletions
This file was deleted.

‎src/test/java/io/github/problem4j/core/ProblemBuilderTest.java‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
package io.github.problem4j.core;
2323

24-
import static io.github.problem4j.core.MapUtils.mapOf;
2524
import static org.assertj.core.api.Assertions.assertThat;
2625
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2726

@@ -154,7 +153,7 @@ void givenNullValueExtension_shouldNotIncludeIt() {
154153
assertThat(problem.getExtensions()).isEmpty();
155154
assertThat(problem.hasExtension("key")).isFalse();
156155
assertThat(problem.getExtensionValue("key")).isNull();
157-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf());
156+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of());
158157
}
159158

160159
@Test
@@ -165,7 +164,7 @@ void givenNullValueExtensionViaVarargs_shouldNotIncludeIt() {
165164
.build();
166165

167166
assertThat(problem.getExtensions()).isEmpty();
168-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf());
167+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of());
169168

170169
assertThat(problem.hasExtension("key1")).isFalse();
171170
assertThat(problem.getExtensionValue("key1")).isNull();
@@ -183,7 +182,7 @@ void givenNullValueExtensionViaObject_shouldNotIncludeIt() {
183182
.build();
184183

185184
assertThat(problem.getExtensions()).isEmpty();
186-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf());
185+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of());
187186

188187
assertThat(problem.hasExtension("key1")).isFalse();
189188
assertThat(problem.getExtensionValue("key1")).isNull();
@@ -210,7 +209,7 @@ void givenMapExtensionWithNullValue_shouldIgnoreNullValue() {
210209
assertThat(problem.getExtensions()).containsExactly("a");
211210
assertThat(problem.hasExtension("a")).isTrue();
212211
assertThat(problem.getExtensionValue("a")).isEqualTo("b");
213-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf("a", "b"));
212+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of("a", "b"));
214213
}
215214

216215
@Test
@@ -230,7 +229,7 @@ void givenVarargWithNullElement_shouldIgnoreNullElement() {
230229
assertThat(problem.getExtensions()).containsExactlyInAnyOrder("a", "b");
231230
assertThat(problem.hasExtension("a")).isTrue();
232231
assertThat(problem.hasExtension("b")).isTrue();
233-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf("a", 1, "b", 2));
232+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of("a", 1, "b", 2));
234233
}
235234

236235
@Test
@@ -251,7 +250,7 @@ void givenCollectionWithNullElement_shouldIgnoreNullElement() {
251250
assertThat(problem.getExtensions()).containsExactlyInAnyOrder("x", "y");
252251
assertThat(problem.hasExtension("x")).isTrue();
253252
assertThat(problem.hasExtension("y")).isTrue();
254-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf("x", "1", "y", "2"));
253+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of("x", "1", "y", "2"));
255254
}
256255

257256
@Test
@@ -297,7 +296,52 @@ void givenAssigningTheSameExtensionLater_shouldOverwriteEarlierValues() {
297296

298297
assertThat(problem.getExtensionValue("k")).isEqualTo("v2");
299298
assertThat(problem.getExtensions()).containsExactly("k");
300-
assertThat(problem.getExtensionMembers()).isEqualTo(mapOf("k", "v2"));
299+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of("k", "v2"));
300+
}
301+
302+
@Test
303+
void givenExtensionSetThenUnsetWithNull_shouldRemoveExtension() {
304+
Problem problem = Problem.builder().extension("name", "Mark").extension("name", null).build();
305+
306+
assertThat(problem.getExtensions()).isEmpty();
307+
assertThat(problem.hasExtension("name")).isFalse();
308+
assertThat(problem.getExtensionValue("name")).isNull();
309+
assertThat(problem.getExtensionMembers()).isEqualTo(Map.of());
310+
}
311+
312+
@Test
313+
void givenExtensionSetThenUnsetViaMap_shouldRemoveExtension() {
314+
Map<String, Object> removals = new HashMap<>();
315+
removals.put("name", null);
316+
317+
Problem problem = Problem.builder().extension("name", "Mark").extensions(removals).build();
318+
319+
assertThat(problem.getExtensions()).isEmpty();
320+
assertThat(problem.hasExtension("name")).isFalse();
321+
}
322+
323+
@Test
324+
void givenExtensionSetThenUnsetViaVarargs_shouldRemoveExtension() {
325+
Problem problem =
326+
Problem.builder()
327+
.extension("name", "Mark")
328+
.extensions(Problem.extension("name", null))
329+
.build();
330+
331+
assertThat(problem.getExtensions()).isEmpty();
332+
assertThat(problem.hasExtension("name")).isFalse();
333+
}
334+
335+
@Test
336+
void givenExtensionSetThenUnsetViaCollection_shouldRemoveExtension() {
337+
Problem problem =
338+
Problem.builder()
339+
.extension("name", "Mark")
340+
.extensions(Collections.singletonList(Problem.extension("name", null)))
341+
.build();
342+
343+
assertThat(problem.getExtensions()).isEmpty();
344+
assertThat(problem.hasExtension("name")).isFalse();
301345
}
302346

303347
@Test

0 commit comments

Comments
 (0)