Add "fallback cases" for relevant enums in :nessie-model
#6634
Conversation
|
@dimas-b WDYT about this one? It is not a great one, but it might "safe our a.." at some point. If you think this could be useful, I'll go ahead an finish it (tests and such) |
api/model/src/main/java/org/projectnessie/model/MergeBehavior.java
Outdated
Show resolved
Hide resolved
snazy
force-pushed
the
fallback-cases-for-enums
branch
2 times, most recently
from
c0b5925
to
c5ca7b3
Compare
snazy
force-pushed
the
fallback-cases-for-enums
branch
from
0d2986b
to
024af6e
Compare
snazy
force-pushed
the
fallback-cases-for-enums
branch
from
eed4268
to
fecce87
Compare
| @@ -399,8 +399,7 @@ public void testInvalidTag(String invalidTagName) { | |||
| .extract() | |||
| .as(NessieError.class) | |||
| .getMessage()) | |||
| .contains( | |||
| "Could not resolve type id 'FOOBAR' as a subtype of `org.projectnessie.model.Reference`")); | |||
| .contains("assignReference.expectedHash: must not be null")); | |||
There was a problem hiding this comment.
Shouldn't this still be something like "Reference type FOOBAR is not supported"? That is to say that we should probably report the same "unsupported" error on the server side regardless of the API method (assignReference.expectedHash: must not be null is specific to ref. assignment, but the real problem is that the ref. type is bogus).
There was a problem hiding this comment.
It's ambiguous with this change to default to TAG
There was a problem hiding this comment.
Ah, right... I guess that default is meaningful only on the client side. If the server receives a reference with a type that it does not know about, that is probably a "bad request" regardless of how JSON serialization is done... WDYT?
There was a problem hiding this comment.
That's difficult. Opted to remove the change for ReferenceType.
api/model/src/main/java/org/projectnessie/model/types/ReferenceTypeIdResolver.java
Outdated
Show resolved
Hide resolved
| } | ||
| return null; | ||
| } catch (IllegalArgumentException e) { | ||
| return TAG; |
There was a problem hiding this comment.
I'm trying to apply your ideas to #6646... Is the intent here to return a "random" enum constant when the text does not match any known enum constant?
There was a problem hiding this comment.
Not a random one, but the "likely best match" / least-common-denominator
snazy
force-pushed
the
fallback-cases-for-enums
branch
from
fecce87
to
b40bfb8
Compare
Adds custom deserializers for all enums in `:nessie-model` to make client & servers "safe" against new enum values. All relevant enums have a `static ENUMTYPE parse(String)` method, which falls back to a hard coded "safe default" value. This apporach is meant to be prepared for emergency cases, when it is strictly necessary to add a new enum value. Or potentially also to let older clients continue to work "longer" against newer API/spec versions. This approach is _NOT_ meant as a workaround to add arbitrary enum values, it should still be avoided wherever and whenever possible to "just add enum value".
snazy
force-pushed
the
fallback-cases-for-enums
branch
from
b40bfb8
to
7f179cc
Compare
snazy
changed the title
:nessie-model:nessie-model
snazy
changed the title
:nessie-model:nessie-model
Adds custom deserializers for all enums in
:nessie-modelto make client & servers "safe" against new enum values. All relevant enums have astatic ENUMTYPE parse(String)method, which falls back to a hard coded "safe default" value.This apporach is meant to be prepared for emergency cases, when it is strictly necessary to add a new enum value. Or potentially also to let older clients continue to work "longer" against newer API/spec versions.
This approach is NOT meant as a workaround to add arbitrary enum values, it should still be avoided wherever and whenever possible to "just add enum value".