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.

Sign up for GitHub

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

[Service Bus] Convert ISO-8601 timestamp string into Date type #9434

Merged
merged 9 commits into from
Jun 19, 2020
Merged

[Service Bus] Convert ISO-8601 timestamp string into Date type #9434

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6e61ce7
Helper method to convert ISO-8601 time into Date type
HarshaNalluru Jun 10, 2020
502d156
createdAt, accessedAt, updatedAt
HarshaNalluru Jun 10, 2020
85d9e14
update Tests
HarshaNalluru Jun 10, 2020
0f2a1cf
API Report
HarshaNalluru Jun 10, 2020
ceb8a36
getDateOrUndefined -> getDate
HarshaNalluru Jun 10, 2020
cd7c057
API report
HarshaNalluru Jun 10, 2020
6f14676
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…
HarshaNalluru Jun 18, 2020
0f852b0
"At" -> "On"
HarshaNalluru Jun 18, 2020
b18fdc6
API Report
HarshaNalluru Jun 18, 2020
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
22 changes: 11 additions & 11 deletions sdk/servicebus/service-bus/review/service-bus.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,12 @@ export { MessagingError }

// @public
export interface NamespaceProperties {
createdOn: string;
createdAt?: Date;
Copy link
Member

@richardpark-msft richardpark-msft Jun 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I did a little tour of the SDK (not just SB) code base and I see 'At' and 'On'. Is this the official proper suffix now?

Copy link
Member Author

Choose a reason for hiding this comment

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

.NET was doing "At". This is to be consistent with that since there seems no reason to deviate.
Both "At" and "On" seem fine to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a guideline to use one or the other, can we check that?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a guideline to use one or the other, can we check that?

I couldn't find that in the guidelines.
But I found an unrelated example which has "lastModifiedDate".
https://azure.github.io/azure-sdk/general_design.html#model-types

Moreover, Storage SDKs are following "On".

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not convinced with changing from on to at
I recall during Event Hubs work that we consistently changed all such properties to use on across all languages. I would recommend we revert the name change and keep the type change in this PR. We can revisit the name later

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'll revert.

messagingSku: string;
messagingUnits: number | undefined;
name: string;
namespaceType: string;
updatedOn: string;
updatedAt?: Date;
}

// @public
Expand Down Expand Up @@ -253,13 +253,13 @@ export interface QueueResponse extends QueueDescription {

// @public
export interface QueueRuntimeInfo {
accessedOn?: string;
createdOn?: string;
accessedAt?: Date;
createdAt?: Date;
messageCount?: number;
messageCountDetails?: MessageCountDetails;
name: string;
sizeInBytes?: number;
updatedOn?: string;
updatedAt?: Date;
}

// @public
Expand Down Expand Up @@ -513,13 +513,13 @@ export interface SubscriptionResponse extends SubscriptionDescription {

// @public
export interface SubscriptionRuntimeInfo {
accessedOn?: string;
createdOn: string;
accessedAt?: Date;
createdAt?: Date;
messageCount: number;
messageCountDetails?: MessageCountDetails;
subscriptionName: string;
topicName: string;
updatedOn: string;
updatedAt?: Date;
}

export { TokenCredential }
Expand Down Expand Up @@ -549,12 +549,12 @@ export interface TopicResponse extends TopicDescription {

// @public
export interface TopicRuntimeInfo {
accessedOn?: string;
createdOn?: string;
accessedAt?: Date;
createdAt?: Date;
name: string;
sizeInBytes?: number;
subscriptionCount?: number;
updatedOn?: string;
updatedAt?: Date;
}

// @public
Expand Down
10 changes: 5 additions & 5 deletions sdk/servicebus/service-bus/src/serializers/namespaceResourceSerializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
deserializeAtomXmlResponse,
serializeToAtomXmlRequest
} from "../util/atomXmlHelper";
import { getInteger, getString } from "../util/utils";
import { getInteger, getString, getDateOrUndefined } from "../util/utils";

/**
* Represents the metadata related to a service bus namespace.
Expand All @@ -19,7 +19,7 @@ export interface NamespaceProperties {
/**
* The time at which the namespace was created.
*/
createdOn: string;
createdAt?: Date;
/**
* The SKU/tier of the namespace.
* "Basic", "Standard" and "Premium"
Expand All @@ -28,7 +28,7 @@ export interface NamespaceProperties {
/**
* The last time at which the namespace was modified.
*/
updatedOn: string;
updatedAt?: Date;
/**
* Name of the namespace.
*/
Expand All @@ -55,9 +55,9 @@ export interface NamespaceProperties {
export function buildNamespace(rawNamespace: any): NamespaceProperties {
const messagingSku = getString(rawNamespace["MessagingSKU"], "messagingSku");
return {
createdOn: getString(rawNamespace["CreatedTime"], "createdOn"),
createdAt: getDateOrUndefined(getString(rawNamespace["CreatedTime"], "createdOn")),
messagingSku: messagingSku,
updatedOn: getString(rawNamespace["ModifiedTime"], "updatedOn"),
updatedAt: getDateOrUndefined(getString(rawNamespace["ModifiedTime"], "updatedOn")),
name: getString(rawNamespace["Name"], "name"),
namespaceType: getString(rawNamespace["NamespaceType"], "namespaceType"),
messagingUnits:
Expand Down
15 changes: 8 additions & 7 deletions sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import {
getRawAuthorizationRules,
getString,
getStringOrUndefined,
MessageCountDetails
MessageCountDetails,
getDateOrUndefined
} from "../util/utils";

/**
Expand Down Expand Up @@ -118,9 +119,9 @@ export function buildQueueRuntimeInfo(rawQueue: any): QueueRuntimeInfo {
sizeInBytes: getIntegerOrUndefined(rawQueue[Constants.SIZE_IN_BYTES]),
messageCount: getIntegerOrUndefined(rawQueue[Constants.MESSAGE_COUNT]),
messageCountDetails: getCountDetailsOrUndefined(rawQueue[Constants.COUNT_DETAILS]),
createdOn: rawQueue[Constants.CREATED_AT],
updatedOn: rawQueue[Constants.UPDATED_AT],
accessedOn: rawQueue[Constants.ACCESSED_AT]
createdAt: getDateOrUndefined(rawQueue[Constants.CREATED_AT]),
updatedAt: getDateOrUndefined(rawQueue[Constants.UPDATED_AT]),
accessedAt: getDateOrUndefined(rawQueue[Constants.ACCESSED_AT])
};
}

Expand Down Expand Up @@ -378,17 +379,17 @@ export interface QueueRuntimeInfo {
/**
* Created at timestamp
*/
createdOn?: string;
createdAt?: Date;

/**
* Updated at timestamp
*/
updatedOn?: string;
updatedAt?: Date;

/**
* Accessed at timestamp
*/
accessedOn?: string;
accessedAt?: Date;

/**
* The entity's message count.
Expand Down
15 changes: 8 additions & 7 deletions sdk/servicebus/service-bus/src/serializers/subscriptionResourceSerializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
getInteger,
getString,
getStringOrUndefined,
MessageCountDetails
MessageCountDetails,
getDateOrUndefined
} from "../util/utils";

/**
Expand Down Expand Up @@ -108,9 +109,9 @@ export function buildSubscriptionRuntimeInfo(rawSubscription: any): Subscription
topicName: getString(rawSubscription[Constants.TOPIC_NAME], "topicName"),
messageCount: getInteger(rawSubscription[Constants.MESSAGE_COUNT], "messageCount"),
messageCountDetails: getCountDetailsOrUndefined(rawSubscription[Constants.COUNT_DETAILS]),
createdOn: getString(rawSubscription[Constants.CREATED_AT], "createdOn"),
updatedOn: getString(rawSubscription[Constants.UPDATED_AT], "updatedOn"),
accessedOn: rawSubscription[Constants.ACCESSED_AT]
createdAt: getDateOrUndefined(getString(rawSubscription[Constants.CREATED_AT], "createdOn")),
updatedAt: getDateOrUndefined(getString(rawSubscription[Constants.UPDATED_AT], "updatedOn")),
accessedAt: getDateOrUndefined(rawSubscription[Constants.ACCESSED_AT])
};
}

Expand Down Expand Up @@ -344,17 +345,17 @@ export interface SubscriptionRuntimeInfo {
/**
* Created at timestamp
*/
createdOn: string;
createdAt?: Date;

/**
* Updated at timestamp
*/
updatedOn: string;
updatedAt?: Date;

/**
* Accessed at timestamp
*/
accessedOn?: string;
accessedAt?: Date;
}

/**
Expand Down
15 changes: 8 additions & 7 deletions sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
getIntegerOrUndefined,
getRawAuthorizationRules,
getString,
getStringOrUndefined
getStringOrUndefined,
getDateOrUndefined
} from "../util/utils";

/**
Expand Down Expand Up @@ -97,9 +98,9 @@ export function buildTopicRuntimeInfo(rawTopic: any): TopicRuntimeInfo {
name: getString(rawTopic[Constants.TOPIC_NAME], "topicName"),
sizeInBytes: getIntegerOrUndefined(rawTopic[Constants.SIZE_IN_BYTES]),
subscriptionCount: getIntegerOrUndefined(rawTopic[Constants.SUBSCRIPTION_COUNT]),
createdOn: rawTopic[Constants.CREATED_AT],
updatedOn: rawTopic[Constants.UPDATED_AT],
accessedOn: rawTopic[Constants.ACCESSED_AT]
createdAt: getDateOrUndefined(rawTopic[Constants.CREATED_AT]),
updatedAt: getDateOrUndefined(rawTopic[Constants.UPDATED_AT]),
accessedAt: getDateOrUndefined(rawTopic[Constants.ACCESSED_AT])
};
}

Expand Down Expand Up @@ -292,17 +293,17 @@ export interface TopicRuntimeInfo {
/**
* Created at timestamp
*/
createdOn?: string;
createdAt?: Date;

/**
* Updated at timestamp
*/
updatedOn?: string;
updatedAt?: Date;

/**
* Accessed at timestamp
*/
accessedOn?: string;
accessedAt?: Date;
}

/**
Expand Down
15 changes: 15 additions & 0 deletions sdk/servicebus/service-bus/src/util/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,21 @@ export function getIntegerOrUndefined(value: any): number | undefined {
return result == NaN ? undefined : result;
}

/**
* @internal
* @ignore
* Helper utility to convert ISO-8601 time into Date type,
* or undefined for invalid values.
* @param value
*/
export function getDateOrUndefined(value: string): Date | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

When does this happen, btw? Like if a queue is newly created?

Copy link
Member Author

@HarshaNalluru HarshaNalluru Jun 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for asking, I had a vague recollection from the last week of testing that the value could be undefined, however after some more testing today, it seems the service always returns a valid timestamp. I'll update this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

try {
return new Date(value);
} catch (error) {
return undefined;
}
}

/**
* @internal
* @ignore
Expand Down
58 changes: 29 additions & 29 deletions sdk/servicebus/service-bus/test/atomManagement.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe("Atom management - Namespace", function(): void {
assert.deepEqualExcluding(
namespaceProperties,
{ messagingSku: "Standard", namespaceType: "Messaging", messagingUnits: undefined } as any,
["_response", "createdOn", "updatedOn", "name"]
["_response", "createdAt", "updatedAt", "name"]
);
});
});
Expand Down Expand Up @@ -415,9 +415,9 @@ describe("Atom management - Authentication", function(): void {
);
assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
});
});
Expand Down Expand Up @@ -558,7 +558,7 @@ describe("Atom management - Authentication", function(): void {
it(`Gets runtime info for existing ${testCase.entityType} entities(multiple) successfully`, async () => {
const response = await getEntitiesRuntimeInfo(testCase.entityType, managementTopic1);
const name = testCase.entityType === EntityType.SUBSCRIPTION ? "subscriptionName" : "name";
const paramsToExclude = ["createdOn", "accessedOn", "updatedOn"];
const paramsToExclude = ["createdAt", "accessedAt", "updatedAt"];
for (const info of response) {
if (info[name] == testCase[1].alwaysBeExistingEntity) {
assert.deepEqualExcluding(info, testCase[1].output, paramsToExclude);
Expand Down Expand Up @@ -1101,9 +1101,9 @@ describe("Atom management - Authentication", function(): void {
should.equal(response.name, managementTopic1, "Topic name mismatch");
assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
});
});
Expand Down Expand Up @@ -1193,9 +1193,9 @@ describe("Atom management - Authentication", function(): void {
);
assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
});
});
Expand Down Expand Up @@ -1413,9 +1413,9 @@ describe("Atom management - Authentication", function(): void {

assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
});
});
Expand Down Expand Up @@ -1585,9 +1585,9 @@ describe("Atom management - Authentication", function(): void {
should.equal(response.name, managementRule1, "Rule name mismatch");
assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
});
});
Expand Down Expand Up @@ -1728,9 +1728,9 @@ describe("Atom management - Authentication", function(): void {

assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
} catch (err) {
checkForValidErrorScenario(err, testCase.output);
Expand Down Expand Up @@ -1899,9 +1899,9 @@ describe("Atom management - Authentication", function(): void {

assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
} catch (err) {
checkForValidErrorScenario(err, testCase.output);
Expand Down Expand Up @@ -1971,9 +1971,9 @@ describe("Atom management - Authentication", function(): void {

assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
} catch (err) {
checkForValidErrorScenario(err, testCase.output);
Expand Down Expand Up @@ -2135,9 +2135,9 @@ describe("Atom management - Authentication", function(): void {

assert.deepEqualExcluding(response, testCase.output, [
"_response",
"createdOn",
"updatedOn",
"accessedOn"
"createdAt",
"updatedAt",
"accessedAt"
]);
} catch (err) {
checkForValidErrorScenario(err, testCase.output);
Expand Down