May put any kind of extra field value

[llbrt]

• adds a method in ServiceException to put an extra field
  value that may be unsafe (array, List, Map, ...)
• adds a unit test

---

This change is 

[alarribeau]

Reviewed 2 of 2 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @clementdenis and @RaHery)

[RaHery]

Is there any particular reason to set this method as protected while the other flavors of putExtraField are public ? This one fulfills exactly the same need as the other.
Same way, suffixing its name by unsafe is really puzzling since it has exactly the same purpose of the others. Plus, this suffix is not really invitation to use it). I would set its signature as the following :

Suggested change

	protected ServiceException putExtraFieldUnsafe(String fieldName, Object value) {
	public ServiceException putExtraField(String fieldName, Object value) {

[llbrt]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @clementdenis and @RaHery)

---

endpoints-framework/src/main/java/com/google/api/server/spi/ServiceException.java, line 167 at r1 (raw file):

Previously, RaHery (Hermann RANGAMANA) wrote…

Is there any particular reason to set this method as protected while the other flavors of putExtraField are public ? This one fulfills exactly the same need as the other.
Same way, suffixing its name by unsafe is really puzzling since it has exactly the same purpose of the others. Plus, this suffix is not really invitation to use it). I would set its signature as the following :

public ServiceException putExtraField(String fieldName, Object value) {

Yes, that's the point:

• it's not as safe as the others because the value may not be immutable
• the dev that want to use it must make more efforts 😈

Another point: the method signature may be ambiguous if we keep the same name with parameter 'Object value'.

[alarribeau]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @clementdenis, @llbrt, and @RaHery)

---

endpoints-framework/src/main/java/com/google/api/server/spi/ServiceException.java, line 160 at r1 (raw file):

   * with the same name was previously set, the old value is replaced by the specified
   * value.<br>
   * This unsafe version accepts any POJO as is: nor defensive copy nor conversion are made.

We should also mention the risk of providing a non-serializable object.

[RaHery]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @clementdenis and @llbrt)

[llbrt]

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @alarribeau and @clementdenis)

---

endpoints-framework/src/main/java/com/google/api/server/spi/ServiceException.java, line 160 at r1 (raw file):

Previously, alarribeau wrote…

We should also mention the risk of providing a non-serializable object.

Javadoc updated.

[alarribeau]

Reviewed 1 of 1 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @clementdenis)

[codecov-io]

Codecov Report

Merging #20 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master      #20      +/-   ##
============================================
+ Coverage     80.96%   80.96%   +<.01%     
- Complexity     1721     1722       +1     
============================================
  Files           159      159              
  Lines          6394     6395       +1     
  Branches        830      830              
============================================
+ Hits           5177     5178       +1     
  Misses          907      907              
  Partials        310      310

Impacted Files	Coverage Δ	Complexity Δ
...va/com/google/api/server/spi/ServiceException.java	80.7% <100%> (+0.34%)	17 <1> (+1)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bbc228...045ed34. Read the comment docs.