Skip to content
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

[Storage] Make the CORS rules nullable #7766

Merged
merged 6 commits into from
Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14546,7 +14546,7 @@ public partial class BlobServiceProperties
/// <summary>
/// The set of CORS rules.
/// </summary>
public System.Collections.Generic.IList<Azure.Storage.Blobs.Models.CorsRule> Cors { get; internal set; }
public System.Collections.Generic.IList<Azure.Storage.Blobs.Models.CorsRule> Cors { get; set; }

/// <summary>
/// The default version to use for requests to the Blob service if an incoming request's version is not specified. Possible values include version 2008-10-27 and all more recent versions
Expand Down Expand Up @@ -14582,7 +14582,6 @@ internal BlobServiceProperties(bool skipInitialization)
Logging = new Azure.Storage.Blobs.Models.Logging();
HourMetrics = new Azure.Storage.Blobs.Models.Metrics();
MinuteMetrics = new Azure.Storage.Blobs.Models.Metrics();
Cors = new System.Collections.Generic.List<Azure.Storage.Blobs.Models.CorsRule>();
DeleteRetentionPolicy = new Azure.Storage.Blobs.Models.RetentionPolicy();
StaticWebsite = new Azure.Storage.Blobs.Models.StaticWebsite();
}
Expand Down
10 changes: 10 additions & 0 deletions sdk/storage/Azure.Storage.Blobs/swagger/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ directive:
}
```

### Make CORS allow null values
It should be possible to pass null for CORS to update service properties without changing existing rules.
``` yaml
directive:
- from: swagger-document
where: $.definitions.BlobServiceProperties
ShivangiReja marked this conversation as resolved.
Show resolved Hide resolved
transform: >
$.properties.Cors["x-az-nullable-array"] = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a note about this new property at the top of https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Common/swagger/Generator/readme.md

I'm trying to keep track of all the extensions we've added.

```

### /?restype=service&comp=stats
``` yaml
directive:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ We support a number of extensions including using the vendor prefix `x-az-`:
- `x-az-skip-path-components`: Whether to skip any path components and always assume a fully formed URL to the resource (this currently must be set to `true`).
- `x-az-include-sync-methods`: Whether to generate support for sync methods. The default value is `false` (this flag should go away soon and always be `true`).
- `x-az-stream`: Whether to generate a non buffered request that takes owhership of the response stream. The default value is `false`.
- `x-az-nullable-array`: Allows list to be null. The default value is `false`.

### Autorest plugin configuration
The AutoRest example at https://github.com/Azure/autorest-extension-helloworld
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ function generateObject(w: IndentWriter, model: IServiceModel, type: IObjectType
w.line(`#pragma warning disable CA1819 // Properties should not return arrays`);
}
w.write(`public ${types.getDeclarationType(property.model, property.required, property.readonly)} ${naming.property(property.clientName)} { get; `);
if (property.readonly || property.model.type === `array`) {
if (!property.isNullable && (property.readonly || property.model.type === `array`)) {
w.write(`internal `);
}
w.line(`set; }`);
Expand All @@ -853,7 +853,7 @@ function generateObject(w: IndentWriter, model: IServiceModel, type: IObjectType
}

// Instantiate nested models if necessary
const nested = (<IProperty[]>Object.values(type.properties)).filter(p => isObjectType(p.model) || (isPrimitiveType(p.model) && (p.model.itemType || p.model.type === `dictionary`)));
const nested = (<IProperty[]>Object.values(type.properties)).filter(p => !p.isNullable && (isObjectType(p.model) || (isPrimitiveType(p.model) && (p.model.itemType || p.model.type === `dictionary`))));
if (nested.length > 0) {
const skipInitName = `skipInitialization`;
if (separator()) { w.line(); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ export interface IProperty {
required?: boolean,
readonly: boolean,
xml?: IXmlSettings,
model: IModelType
model: IModelType,
isNullable?: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be isNullable? or can it just be isNullable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really wanted to not have this as an optional parameter but since TypeScript doesn't assign default values for primitive types, I had to make it as optional (microsoft/TypeScript#5113)

If I'd make isNullable required then we would have to set isNullable when we add headers and body in createResponseType() method.

We can set false in those cases but do we need to set isNullable when creating a response type?

}

export interface IXmlSettings {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,19 @@ function createObjectType(project: IProject, name: string, swagger: any, locatio
unsupported(() => def[`x-ms-client-flatten`], `${location}.properties['${name}']`);
unsupported(() => def[`x-ms-mutability`], `${location}.properties['${name}']`);

let isNullable: boolean|undefined = def[`x-az-nullable-array`];
if (isNullable === undefined) {
isNullable = false;
}

properties[name] = {
name,
clientName: optional(() => def[`x-ms-client-name`], name),
description: def.description,
readonly: true,
xml: def.xml || { },
model: createType(project, name, def, `${location}.properties['${name}']`)
model: createType(project, name, def, `${location}.properties['${name}']`),
isNullable: isNullable
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6795,7 +6795,7 @@ public partial class FileServiceProperties
/// <summary>
/// The set of CORS rules.
/// </summary>
public System.Collections.Generic.IList<Azure.Storage.Files.Models.CorsRule> Cors { get; internal set; }
public System.Collections.Generic.IList<Azure.Storage.Files.Models.CorsRule> Cors { get; set; }

/// <summary>
/// Creates a new FileServiceProperties instance
Expand All @@ -6815,7 +6815,6 @@ internal FileServiceProperties(bool skipInitialization)
{
HourMetrics = new Azure.Storage.Files.Models.Metrics();
MinuteMetrics = new Azure.Storage.Files.Models.Metrics();
Cors = new System.Collections.Generic.List<Azure.Storage.Files.Models.CorsRule>();
}
}

Expand Down
10 changes: 10 additions & 0 deletions sdk/storage/Azure.Storage.Files/swagger/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ directive:
}
```

### Make CORS allow null values
It should be possible to pass null for CORS to update service properties without changing existing rules.
``` yaml
directive:
- from: swagger-document
where: $.definitions.FileServiceProperties
transform: >
$.properties.Cors["x-az-nullable-array"] = true;
```

### /?comp=list
``` yaml
directive:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3721,7 +3721,7 @@ public partial class QueueServiceProperties
/// <summary>
/// The set of CORS rules.
/// </summary>
public System.Collections.Generic.IList<Azure.Storage.Queues.Models.CorsRule> Cors { get; internal set; }
public System.Collections.Generic.IList<Azure.Storage.Queues.Models.CorsRule> Cors { get; set; }

/// <summary>
/// Creates a new QueueServiceProperties instance
Expand All @@ -3742,7 +3742,6 @@ internal QueueServiceProperties(bool skipInitialization)
Logging = new Azure.Storage.Queues.Models.Logging();
HourMetrics = new Azure.Storage.Queues.Models.Metrics();
MinuteMetrics = new Azure.Storage.Queues.Models.Metrics();
Cors = new System.Collections.Generic.List<Azure.Storage.Queues.Models.CorsRule>();
}
}

Expand Down
10 changes: 10 additions & 0 deletions sdk/storage/Azure.Storage.Queues/swagger/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,16 @@ directive:
}
```

### Make CORS allow null values
It should be possible to pass null for CORS to update service properties without changing existing rules.
``` yaml
directive:
- from: swagger-document
where: $.definitions.QueueServiceProperties
transform: >
$.properties.Cors["x-az-nullable-array"] = true;
```

### QueueServiceStatistics
``` yaml
directive:
Expand Down