-
Notifications
You must be signed in to change notification settings - Fork 134
Add YAML/JSON backward compatibility for xDS resource files #1324
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| import java.io.IOException; | ||
| import java.lang.reflect.Method; | ||
| import java.lang.reflect.Modifier; | ||
| import java.util.function.Function; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| import org.curioswitch.common.protobuf.json.MessageMarshaller; | ||
|
|
@@ -185,6 +186,8 @@ public <T extends Message> void push( | |
| final JsonNode jsonNode = Jackson.readTree(jsonText); | ||
| if (create) { | ||
| change = Change.ofJsonPatch(fileName, null, jsonNode); | ||
| } else if (fileName.endsWith(".yaml")) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that only
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's correct because this is for compatibility. |
||
| change = Change.ofYamlUpsert(fileName, jsonNode); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } else { | ||
| change = Change.ofJsonUpsert(fileName, jsonNode); | ||
| } | ||
|
|
@@ -236,8 +239,8 @@ public <T extends Message> void update(StreamObserver<T> responseObserver, Strin | |
| public <T extends Message> void update(StreamObserver<T> responseObserver, String group, | ||
| String resourceName, String fileName, String summary, T resource, | ||
| Author author) { | ||
| updateOrDelete(responseObserver, group, resourceName, fileName, | ||
| () -> push(responseObserver, group, resourceName, fileName, | ||
| updateOrDelete(responseObserver, group, resourceName, fileName, resolvedFileName -> | ||
| () -> push(responseObserver, group, resourceName, resolvedFileName, | ||
| summary, resource, author, false)); | ||
| } | ||
|
|
||
|
|
@@ -248,10 +251,10 @@ public void delete(StreamObserver<Empty> responseObserver, String group, | |
|
|
||
| public void delete(StreamObserver<Empty> responseObserver, String group, | ||
| String resourceName, String fileName, String summary, Author author) { | ||
| final Runnable deleteTask = () -> | ||
| updateOrDelete(responseObserver, group, resourceName, fileName, resolvedFileName -> () -> | ||
| commandExecutor.execute(Command.push(author, XDS_CENTRAL_DOGMA_PROJECT, group, | ||
| Revision.HEAD, summary, "", Markup.PLAINTEXT, | ||
| ImmutableList.of(Change.ofRemoval(fileName)))) | ||
| ImmutableList.of(Change.ofRemoval(resolvedFileName)))) | ||
| .handle((unused, cause) -> { | ||
| if (cause != null) { | ||
| responseObserver.onError( | ||
|
|
@@ -261,14 +264,17 @@ public void delete(StreamObserver<Empty> responseObserver, String group, | |
| responseObserver.onNext(Empty.getDefaultInstance()); | ||
| responseObserver.onCompleted(); | ||
| return null; | ||
| }); | ||
| updateOrDelete(responseObserver, group, resourceName, fileName, deleteTask); | ||
| })); | ||
| } | ||
|
|
||
| public void updateOrDelete(StreamObserver<?> responseObserver, String group, String resourceName, | ||
| String fileName, Runnable task) { | ||
| String fileName, Function<String, Runnable> taskProvider) { | ||
| final Repository repository = xdsProject.repos().get(group); | ||
| repository.find(Revision.HEAD, fileName, FIND_ONE_WITHOUT_CONTENT).handle((entries, cause) -> { | ||
| // Search for both the requested filename and its alternative extension (.json ↔ .yaml) | ||
| // to support files that may have been written in either format. | ||
| final String altFileName = alternativeFileName(fileName); | ||
| repository.find(Revision.HEAD, fileName + ',' + altFileName, FIND_ONE_WITHOUT_CONTENT) | ||
| .handle((entries, cause) -> { | ||
| if (cause != null) { | ||
| responseObserver.onError(cause); | ||
| return null; | ||
|
|
@@ -279,8 +285,19 @@ public void updateOrDelete(StreamObserver<?> responseObserver, String group, Str | |
| .asRuntimeException()); | ||
| return null; | ||
| } | ||
| task.run(); | ||
| final String resolvedFileName = entries.keySet().iterator().next(); | ||
| taskProvider.apply(resolvedFileName).run(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question) If
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Fixed. 😉 |
||
| return null; | ||
| }); | ||
| } | ||
|
|
||
| private static String alternativeFileName(String fileName) { | ||
| if (fileName.endsWith(".json")) { | ||
| return fileName.substring(0, fileName.length() - 5) + ".yaml"; | ||
| } | ||
| if (fileName.endsWith(".yaml")) { | ||
| return fileName.substring(0, fileName.length() - 5) + ".json"; | ||
| } | ||
| return fileName; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.