Case-insensitive Discovery message type names by andreamlin · Pull Request #1956 · googleapis/gapic-generator

andreamlin · 2018-04-30T16:58:34Z

Windows NTFS is case-insensitive, so this PR escapes DiscoveryField identifiers that are the same, disregarding character casing.

Introduce global table of DiscoveryFields to prevent duplicate creation of DiscoveryFields from the same Schema.
Constructor for DiscoveryFields enforces unique, case-insensitive identifiers for message-type Schemas.
Refactor SchemaTypeName* utils to use DiscoveryFields instead of Schemas. This allows the TypeName utils to use the DiscoveryField's unique identifiers.

andreamlin · 2018-04-30T18:57:28Z

PTAL

codecov-io · 2018-04-30T19:09:13Z

Codecov Report

Merging #1956 into master will decrease coverage by 0.07%.
The diff coverage is 74.28%.

@@             Coverage Diff              @@
##             master    #1956      +/-   ##
============================================
- Coverage     86.01%   85.94%   -0.08%     
- Complexity     4933     4949      +16     
============================================
  Files           452      452              
  Lines         19655    19719      +64     
  Branches       2055     2076      +21     
============================================
+ Hits          16906    16947      +41     
- Misses         2065     2076      +11     
- Partials        684      696      +12

Impacted Files	Coverage Δ	Complexity Δ
...va/com/google/api/codegen/util/CommonAcronyms.java	`90.9% <ø> (ø)`	`17 <0> (ø)`	⬇️
...i/codegen/transformer/SchemaTypeFormatterImpl.java	`36.84% <0%> (ø)`	`5 <0> (ø)`	⬇️
...i/codegen/transformer/SchemaTypeNameConverter.java	`66.66% <0%> (ø)`	`5 <0> (ø)`	⬇️
...degen/discogapic/transformer/DiscoGapicParser.java	`91.52% <100%> (+0.45%)`	`22 <5> (+1)`	⬆️
...oogle/api/codegen/config/DiscoveryMethodModel.java	`82.1% <100%> (-0.37%)`	`40 <0> (ø)`
.../java/com/google/api/codegen/util/SymbolTable.java	`100% <100%> (ø)`	`10 <2> (+1)`	⬆️
...er/java/JavaDiscoGapicSchemaToViewTransformer.java	`95.91% <100%> (+0.04%)`	`16 <7> (-3)`	⬇️
...r/java/JavaDiscoGapicRequestToViewTransformer.java	`94.79% <100%> (+0.02%)`	`18 <0> (ø)`	⬇️
...a/JavaDiscoGapicResourceNameToViewTransformer.java	`100% <100%> (ø)`	`15 <0> (ø)`	⬇️
...oogle/api/codegen/transformer/SchemaTypeTable.java	`70% <25%> (-0.5%)`	`26 <1> (ø)`
... and 7 more

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 a8f6389...cb9767e. Read the comment docs.

src/main/java/com/google/api/codegen/config/DiscoveryField.java

  private final Schema originalSchema;
-  private final DiscoApiModel apiModel;
+
+  // Unformatted type name for this Field. For message-type Fields, this will be globally unique.


src/main/java/com/google/api/codegen/discovery/Method.java

  public abstract String httpMethod();

-  /** @return the ID. */
+  /** @return the ID. This is unique. */


src/main/java/com/google/api/codegen/transformer/SchemaTypeTable.java


  @Override
-  public String getNicknameFor(Schema type) {
+  public String getNicknameFor(DiscoveryField type) {


andreamlin · 2018-05-01T21:17:13Z

Made adjustments. PTAL

src/main/java/com/google/api/codegen/discovery/Schema.java

+   *     content but different parents will still have different hashCodes.
+   */
+  @Override
+  public int hashCode() {


src/main/java/com/google/api/codegen/config/DiscoveryField.java

+              .compareTo(o2.getValue().getDefaultPackageName());
+        }
+      };
+  private static Map<Map.Entry<Schema, DiscoApiModel>, DiscoveryField> globalObjects =


garrettjonesgoogle · 2018-05-02T15:46:32Z

@pongad in case I can't finish this review in a timely fashion - could you take a look too?

andreamlin · 2018-05-02T18:19:26Z

FIXED things. PTAL

src/main/java/com/google/api/codegen/config/DiscoveryField.java

+    if (isTopLevelSchema(schema)) {
+      // Within this namespace, get a unique name for this message-type schema.
+      SchemaNamer messageNamer;
+      if (namespaceToSchemaNamer.containsKey(namespace)) {


src/main/java/com/google/api/codegen/config/DiscoveryField.java

-    Preconditions.checkNotNull(schema);
-    Preconditions.checkNotNull(rootApiModel);
-    return new DiscoveryField(schema, rootApiModel);
+  public static synchronized DiscoveryField create(Schema schema, DiscoApiModel rootApiModel) {


src/main/java/com/google/api/codegen/config/DiscoveryField.java

+    private Map<Schema, String> messageNames = new TreeMap<>(messageSchemaComparator);
+
+    String getSchemaName(Schema schema, String basename) {
+      if (messageNames.containsKey(schema)) {


src/main/java/com/google/api/codegen/config/DiscoveryField.java

+      return Objects.equals(
+              getApiModel().getDefaultPackageName(),
+              (otherModelData.getApiModel().getDefaultPackageName()))
+          && getSchema().hashCode() == otherModelData.getSchema().hashCode();


src/main/java/com/google/api/codegen/discovery/Schema.java

+   *     content but different parents will still have different hashCodes.
+   */
+  @Override
+  public int hashCode() {


src/main/java/com/google/api/codegen/util/SymbolTable.java

+   * @param comparator function to determine equality on Strings
+   */
+  public SymbolTable(Comparator<String> comparator) {
+    symbolTable = new ConcurrentSkipListSet<>(comparator);


andreamlin · 2018-05-02T21:38:38Z

Made fixes. PTAL

src/main/java/com/google/api/codegen/discovery/Schema.java

+   *     content but different parents will still have different hashCodes.
+   */
+  @Override
+  public int hashCode() {


andreamlin changed the title ~~case insensitive filenames (WIP)~~ Case-insensitive Discovery message type names Apr 30, 2018

andreamlin requested a review from garrettjonesgoogle April 30, 2018 18:57

andreamlin mentioned this pull request Apr 30, 2018

GAPIC Compute java library googleapis/google-cloud-java#3100

Merged

garrettjonesgoogle reviewed Apr 30, 2018

View reviewed changes

andreamlin force-pushed the caseinsensitve4 branch from 6d505d4 to efe3df9 Compare May 1, 2018 18:45

garrettjonesgoogle reviewed May 2, 2018

View reviewed changes

garrettjonesgoogle requested a review from pongad May 2, 2018 15:45

pongad reviewed May 2, 2018

View reviewed changes

andreamlin added 16 commits May 3, 2018 23:33

adding files

3887e44

cleanup

b46a535

clean up

07444fd

cleanup

793102b

naming method'

0b05e58

comments

8e2f9cc

comments

b3f508b

debug

27c2d9a

remove debug

06afdb3

preconditions check art for getDefaultPackageName

9710e56

remove preconditions checks:

0d59c2b

remove more preconditions checks

a36f98b

make message type schema names unique within their namespaces

13e668d

add comment

58e7609

equals() override

a2cf86b

use private class for schema and apimodel inside DiscoveryField

b331f43

andreamlin added 5 commits May 3, 2018 23:33

remove extra Preconditions.checkNotNull

97a9aed

use computeIfAbsent

092b06a

more computeIfAbsent

ee78d5a

ModelData.equals()

7fb97ae

just TreeSet

9acfd63

andreamlin force-pushed the caseinsensitve4 branch from 90cb03e to 9acfd63 Compare May 4, 2018 06:34

pongad approved these changes May 4, 2018

View reviewed changes

src/main/java/com/google/api/codegen/discovery/Schema.java

* content but different parents will still have different hashCodes.

*/

@Override

public int hashCode() {

This comment was marked as spam.

Sign in to view

andreamlin and others added 2 commits May 4, 2018 11:19

remove globalObjects talbe in DiscoveryFIeld

7ac21b3

Merge branch 'master' into caseinsensitve4

cb9767e

andreamlin merged commit 4362afd into googleapis:master May 4, 2018

Conversation

andreamlin commented Apr 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andreamlin commented Apr 30, 2018

Uh oh!

codecov-io commented Apr 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

andreamlin commented May 1, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

garrettjonesgoogle commented May 2, 2018

Uh oh!

andreamlin commented May 2, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

andreamlin commented May 2, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andreamlin commented Apr 30, 2018 •

edited

Loading

codecov-io commented Apr 30, 2018 •

edited

Loading