Azure-sdk-for-java: [API review] Auto-gen code for file

Created on 3 Oct 2019  ·  7Comments  ·  Source: Azure/azure-sdk-for-java

  • [x] Rename FileHTTPHeader -> FileHttpHeaders

  • [x] In FileHTTPHeader, the API has pattern of getFile(), however, in FileProperties, it is get(), do we want to align those? Drop the File from getter and setter?

  • [x] Is it necessary to name this as Ntfs to inform the user of the file system format? Can this just be FileAttributes?

  • [x] Can user set the last modified time? Isn't it set in the service when the user updates something? ShareProperties

  • [x] In Range, since end is optional, should this return Long instead of long?

  • [x] FileProperty is not used anywhere and we have a hand craft FileProperties. Do we still need the FileProperty?

Client Storage blocking-release

All 7 comments

  • Sounds good
  • I think this is a swagger question
  • I think this is useful to disambiguate with azure file properties, etc. Otherwise it becomes unclear why we have FileAttributes and FileProperties
  • User can't set the LMT on the service. ShareProperties is auto-gen, so we can't fix that as easily.
  • I am seeing that FileRange.end() does return Long. Is there a different Range object I'm getting it confused with?
  • I'm seeing it is used in FileRef and FileItem?

Regarding: Rename FileHTTPHeader -> FileHttpHeaders

The model FileHTTPHeaders is generated using x-ms-parameter-grouping attribute in File swagger.

The word HTTP in FileHttpHeaders acronym and based on Java guideline it should be Http. I don’t see same guideline in other languages, so it’s better to add this in File swagger transformation layer for Java not in public swagger.

Action item for Java SDK is: Identify the transformation syntax to rename x-ms-parameter-grouping attribute.

Update: Oct/15: This is addressed in https://github.com/Azure/azure-sdk-for-java/pull/5843, the swagger changes are in the transformation README file.

Regarding: In FileHTTPHeader, the API has pattern of getFile*(), however, in FileProperties, it is get*(), do we want to align those? Drop the File from getter and setter?

As Rick mentioned this is a swagger question, the getter and setter names are generated by prepending get and set in-front of each properties in FileHTTPHeaders model, today in swagger properties of FileHTTPHeaders has the word “file” in it. If we agree on removing the “file” from getters and setters then we need to remove the same word from swagger.

Update Oct/15: The discussion to remove/keep the word file is still in progress. If we need to remove it then transformations like following needs to be added in transformation file:

- from: swagger-document
  where: $.definitions
  transform: >    
      if (!$.FileContentEncoding) {
          const clientName = $.FileContentEncoding["x-ms-client-name"]; // currently "fileContentEncoding"
           if (clientName) {
               $.FileContentEncoding["x-ms-client-name"] = "contentEncoding";
           }
      }

Addressed in https://github.com/Azure/azure-sdk-for-java/pull/5868

Regarding: In Range, since end is optional, should this return Long instead of long?

In request, service allows us to put null value for FileRange::end property. Refer Rest API doc.
In response, service always return the end of the range. It is required field.

So we'd better to leave as it is.

Update Oct/15: This is NOP, see above comments.

Regarding: FileProperty is not used anywhere and we have a hand craft FileProperties. Do we still need the FileProperty?

The response body of API to “List files and directories” includes an array describing each file, such a description contains two fields - file name and size of the file. The “FileProperty” is defined in swagger to hold this“file size”, I believe it’s designed in this way to accommodate more properties in future (e.g. last modified time of file).

The “FileProperties” is something we wrote in our convenience layer to represents result of - Get File property API, service returns properties through headers and in convince layer we move them in this model.

So, FileProperty and FileProperties are different. We cannot remove any one of them.

Update Oct/15: This is NOP, see above comments.

We're close to closing this issue, the last pending item under discussion is removing/keeping the word file in properties of FileHttpHeader model. https://github.com/Azure/azure-sdk-for-java/issues/5677#issuecomment-540785823

\\cc @joshfree

All work items are addressed, issue can be closed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

christopheranderson picture christopheranderson  ·  3Comments

srnagar picture srnagar  ·  4Comments

jordanjennings picture jordanjennings  ·  5Comments

hemanttanwar picture hemanttanwar  ·  3Comments

buchizo picture buchizo  ·  4Comments