[ggj]fix: handle special characters in string object value #72

xiaozhenliu-gg5 · 2020-06-24T05:31:44Z

resolve left comments in #61

write manually the escaper to help handle special characters.
add unit tests to cover special/unicode strings.

more details can be found in the design doc, investigation is appended.

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

src/test/java/com/google/api/generator/engine/ast/StringObjectValueTest.java

…ator-java into fix-string-object

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

…ator-java into fix-string-object

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

WORKSPACE

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

src/main/java/com/google/api/generator/engine/escaper/StringValueEscaper.java

src/test/java/com/google/api/generator/engine/ast/StringObjectValueTest.java

…ator-java into fix-string-object

xiaozhenliu-gg5 · 2020-07-10T22:59:48Z

StringValueEscaper is moved to StringObjectValue class as a nested class, EscaperException is added in escaper package, which will be shared by the commentEscaperthat would be implemented.

miraleung · 2020-07-13T21:14:55Z

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

+    public StringObjectValue build() {
+      // `\"` is added to the escaped string value for interpreting it correctly in file.
+      String value =
+          String.format("\"%s\"", StringValueEscaper.getInstance().escape(autoBuild().value()));


Instead of calling autoBuild twice, what about just using value()? IIRC we discussed this in a prior review.

right, we had this discussion, here we cannot use value() because it's not static, Builder class has static context.

One way to avoid calling autoBuild() twice is to add a "private" value() method to the Builder class. Please see the other examples in this codebase.

Right! Thank you for the advice, also found this doc that illustrate how/why it works. Learned from this PR!

src/main/java/com/google/api/generator/engine/escaper/BUILD.bazel

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

miraleung · 2020-07-13T21:17:46Z

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

+      return escaper.escape(sourceString);
+    }
+
+    public static Escaper getInstance() {


Consider whether this method is still needed.

yes! This is basically for wrapping the escape() in as a static method so that we can call it from the static context (class Builder).

Could you please try calling escape() without getInstance() and consider whether getInstance() is still needed?

sorry if I confused you, but I did try remove the getInstance()and directly call StringValueEscaper.escape(value) and it throws error of cannot calling a non-static method from static context. since the override escape() is not static.

String value = String.format("\"%s\"", StringValueEscaper.escape(value()));

please LMK if I did something wrong.

But this works, thank you!

String value = String.format("\"%s\"", StringValueEscaper.escaper.escape(value()));

TWIL: if the member or constructor is declared private, then access is permitted if and only if it occurs within the body of the top level class that encloses the declaration of the member or constructor.

miraleung · 2020-07-14T23:51:42Z

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java

+    public StringObjectValue build() {
+      // `\"` is added to the escaped string value for interpreting it correctly in file.
+      String value =
+          String.format("\"%s\"", StringValueEscaper.getInstance().escape(autoBuild().value()));


One way to avoid calling autoBuild() twice is to add a "private" value() method to the Builder class. Please see the other examples in this codebase.

…ator-java into fix-string-object

…onfig to v0.8.0 (#72) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [com.google.cloud:google-cloud-shared-config](https://togithub.com/googleapis/java-shared-config) | minor | `0.6.0` -> `0.8.0` | --- ### Release Notes <details> <summary>googleapis/java-shared-config</summary> ### [`v0.8.0`](https://togithub.com/googleapis/java-shared-config/blob/master/CHANGELOG.md#080-httpswwwgithubcomgoogleapisjava-shared-configcomparev070v080-2020-06-10) [Compare Source](https://togithub.com/googleapis/java-shared-config/compare/v0.7.0...v0.8.0) ##### Features - revert "feat: mark auto-value-annotations scope as provided" ([#154](https://www.github.com/googleapis/java-shared-config/issues/154)) ([88afb4e](https://www.github.com/googleapis/java-shared-config/commit/88afb4e7c57cb6e00929c098135314a926d9da30)) ### [`v0.7.0`](https://togithub.com/googleapis/java-shared-config/blob/master/CHANGELOG.md#070-httpswwwgithubcomgoogleapisjava-shared-configcomparev060v070-2020-06-10) [Compare Source](https://togithub.com/googleapis/java-shared-config/compare/v0.6.0...v0.7.0) ##### Features - mark auto-value-annotations scope as provided ([#151](https://www.github.com/googleapis/java-shared-config/issues/151)) ([44ea4cb](https://www.github.com/googleapis/java-shared-config/commit/44ea4cbbf92b4ad35ffaffb7a01a1bce05063daf)) ##### Bug Fixes - lock the google-java-format version used by formatter plugin ([#149](https://www.github.com/googleapis/java-shared-config/issues/149)) ([d58c054](https://www.github.com/googleapis/java-shared-config/commit/d58c05437a4ea8767db2bebfcc405ec77aeb9705)) </details> --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#googleapis/java-shared-dependencies).

fix string object value

4be62b0

xiaozhenliu-gg5 requested review from miraleung and summer-ji-eng June 24, 2020 05:32

clean up

3bf49b7

summer-ji-eng reviewed Jun 24, 2020

View reviewed changes

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java Outdated Show resolved Hide resolved

summer-ji-eng approved these changes Jun 24, 2020

View reviewed changes

miraleung suggested changes Jun 24, 2020

View reviewed changes

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java Outdated Show resolved Hide resolved

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java Outdated Show resolved Hide resolved

xiaozhenliu-gg5 commented Jun 24, 2020

View reviewed changes

src/test/java/com/google/api/generator/engine/ast/StringObjectValueTest.java Outdated Show resolved Hide resolved

Merge branch 'master' into fix-string-object

af811dd

xiaozhenliu-gg5 requested a review from miraleung June 24, 2020 19:25

xiaozhenliu-gg5 added 2 commits June 24, 2020 13:25

clean

b66a93a

Merge branch 'fix-string-object' of github.com:googleapis/gapic-gener…

4299343

…ator-java into fix-string-object

miraleung suggested changes Jun 24, 2020

View reviewed changes

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java Outdated Show resolved Hide resolved

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java Outdated Show resolved Hide resolved

xiaozhenliu-gg5 added 9 commits June 24, 2020 22:10

add double quote to value

386245b

clean

62dcf6b

Merge branch 'master' into fix-string-object

48e90af

add unit tests

249a1e7

Merge branch 'master' into fix-string-object

632ff0a

Merge branch 'fix-string-object' of github.com:googleapis/gapic-gener…

6be5785

…ator-java into fix-string-object

clean

0f80007

assignment expression test

a8f9118

clean

eea1a2c

xiaozhenliu-gg5 requested a review from miraleung June 25, 2020 22:25

clean

b645148

miraleung suggested changes Jun 26, 2020

View reviewed changes

src/main/java/com/google/api/generator/engine/ast/StringObjectValue.java Outdated Show resolved Hide resolved

miraleung reviewed Jun 26, 2020

View reviewed changes

WORKSPACE Outdated Show resolved Hide resolved

xiaozhenliu-gg5 added 2 commits June 26, 2020 12:12

Merge branch 'master' into fix-string-object

1f350ea

Merge branch 'master' into fix-string-object

0f151e4

xiaozhenliu-gg5 requested a review from miraleung June 26, 2020 20:08

Merge branch 'master' into fix-string-object

feda24b

miraleung suggested changes Jul 9, 2020

View reviewed changes

xiaozhenliu-gg5 added 9 commits July 10, 2020 09:49

escaper

07c13da

stringvalue escaper

c0d59e8

fix

d727013

Merge branch 'master' into fix-string-object

011d9f0

Merge branch 'fix-string-object' of github.com:googleapis/gapic-gener…

97b9e20

…ator-java into fix-string-object

fix

0f95a20

Merge branch 'master' into fix-string-object

c942d27

Merge branch 'fix-string-object' of github.com:googleapis/gapic-gener…

7c40abb

…ator-java into fix-string-object

clean up

7b08f8a

xiaozhenliu-gg5 changed the title ~~[ggj]fix: string object value unit test~~ [ggj]fix: handle special characters in string object value Jul 10, 2020

xiaozhenliu-gg5 requested a review from miraleung July 10, 2020 22:59

miraleung suggested changes Jul 13, 2020

View reviewed changes

xiaozhenliu-gg5 added 2 commits July 13, 2020 15:59

remove unneeded dependency

5b7dc67

clean

c4e73c0

xiaozhenliu-gg5 requested a review from miraleung July 13, 2020 23:18

Merge branch 'master' into fix-string-object

cd95779

miraleung suggested changes Jul 14, 2020

View reviewed changes

xiaozhenliu-gg5 added 4 commits July 14, 2020 18:22

Merge branch 'master' into fix-string-object

71686d3

fix

e6e3b66

clean

ea632ae

Merge branch 'fix-string-object' of github.com:googleapis/gapic-gener…

e1aa324

…ator-java into fix-string-object

xiaozhenliu-gg5 requested a review from miraleung July 15, 2020 01:32

miraleung approved these changes Jul 16, 2020

View reviewed changes

Merge branch 'master' into fix-string-object

24017ed

xiaozhenliu-gg5 merged commit 5f44411 into master Jul 16, 2020

miraleung deleted the fix-string-object branch July 25, 2020 09:57

suztomo pushed a commit that referenced this pull request Mar 21, 2023

deps: update dependency io.grpc:grpc-bom to v1.25.0 (#72)

88ab7d8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ggj]fix: handle special characters in string object value #72

[ggj]fix: handle special characters in string object value #72

xiaozhenliu-gg5 commented Jun 24, 2020 •

edited

Loading

xiaozhenliu-gg5 commented Jul 10, 2020

miraleung Jul 13, 2020

xiaozhenliu-gg5 Jul 13, 2020 •

edited

Loading

miraleung Jul 14, 2020

xiaozhenliu-gg5 Jul 15, 2020

miraleung Jul 13, 2020

xiaozhenliu-gg5 Jul 13, 2020

miraleung Jul 14, 2020

xiaozhenliu-gg5 Jul 15, 2020

xiaozhenliu-gg5 Jul 15, 2020 •

edited

Loading

xiaozhenliu-gg5 Jul 15, 2020

miraleung Jul 14, 2020

[ggj]fix: handle special characters in string object value #72

[ggj]fix: handle special characters in string object value #72

Conversation

xiaozhenliu-gg5 commented Jun 24, 2020 • edited Loading

xiaozhenliu-gg5 commented Jul 10, 2020

Choose a reason for hiding this comment

xiaozhenliu-gg5 Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiaozhenliu-gg5 Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiaozhenliu-gg5 commented Jun 24, 2020 •

edited

Loading

xiaozhenliu-gg5 Jul 13, 2020 •

edited

Loading

xiaozhenliu-gg5 Jul 15, 2020 •

edited

Loading