mgmt-core, support systemData for Azure resource #18303

weidongxu-microsoft · 2020-12-22T05:25:57Z

For new Azure resource in mgmt, by default they need to support SystemData.
https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/common-api-contracts.md

Specs here https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v2/types.json#L498-L550

Related PR on generator Azure/autorest.java#938

sdk/core/azure-core-management/src/main/java/com/azure/core/management/CreatedByType.java

weidongxu-microsoft · 2021-02-25T08:05:43Z

sdk/core/azure-core-management/src/main/java/com/azure/core/management/SystemData.java

+    public CreatedByType createdByType() {
+        return this.createdByType;
+    }
+
+    /**
+     * Get the timestamp of resource creation (UTC).
+     *
+     * @return the timestamp of resource creation (UTC).
+     */
+    public OffsetDateTime createdAt() {
+        return this.createdAt;
+    }
+
+    /**
+     * Get the identity that last modified the resource.
+     *
+     * @return the identity that last modified the resource.
+     */
+    public String lastModifiedBy() {
+        return this.lastModifiedBy;
+    }
+
+    /**
+     * Get the type of identity that last modified the resource.
+     *
+     * @return the type of identity that last modified the resource.
+     */
+    public CreatedByType lastModifiedByType() {
+        return this.lastModifiedByType;
+    }
+


@srnagar
Let me know if you have concern with the class name CreatedByType. It is defined by swagger. But it might be not very appropriate (as it is returned by createdByType() and lastModifiedByType(), more like IdentityType?).

However IdentityType would cause quite some naming conflicts with other swagger types (though not too bad as they are under different namespace).

Also appreciate suggestion on method names in SystemData. They seems a bit off (but I do not have better idea).

IdentityType sounds like a better name than CreatedByType as it is used for lastModifiedByType too.

SystemData seems very generic. Is this name finalized? Can we change this to AzureResourceSystemData?

The method names look fine but just want to check, do we prefix get for getter methods in other arm model types?

Got it. Another candidate could be ResourceAuthorIdentityType.

It is generic. The class is intended to be returned by every ARM resources (resource group, virtual network, disk, storage account, virtual machine, etc., well, all) in future. It is already in com.azure.core.management namespace. On the other hand from its name, it does seems belong to something of operating system (so far, I didn't find any existing SystemData class that going to be confused with this). Maybe ResourceSystemData would be better. Let me check e.g. how python or .NET names it.

It is still used as POJO for ARM, so like Resource class, it does not have get prefix.

srnagar · 2021-02-26T09:05:56Z

sdk/core/azure-core-management/src/main/java/com/azure/core/management/SystemData.java

+    public CreatedByType createdByType() {
+        return this.createdByType;
+    }
+
+    /**
+     * Get the timestamp of resource creation (UTC).
+     *
+     * @return the timestamp of resource creation (UTC).
+     */
+    public OffsetDateTime createdAt() {
+        return this.createdAt;
+    }
+
+    /**
+     * Get the identity that last modified the resource.
+     *
+     * @return the identity that last modified the resource.
+     */
+    public String lastModifiedBy() {
+        return this.lastModifiedBy;
+    }
+
+    /**
+     * Get the type of identity that last modified the resource.
+     *
+     * @return the type of identity that last modified the resource.
+     */
+    public CreatedByType lastModifiedByType() {
+        return this.lastModifiedByType;
+    }
+


IdentityType sounds like a better name than CreatedByType as it is used for lastModifiedByType too.

srnagar · 2021-02-26T09:33:54Z

sdk/core/azure-core-management/src/main/java/com/azure/core/management/SystemData.java

+    public CreatedByType createdByType() {
+        return this.createdByType;
+    }
+
+    /**
+     * Get the timestamp of resource creation (UTC).
+     *
+     * @return the timestamp of resource creation (UTC).
+     */
+    public OffsetDateTime createdAt() {
+        return this.createdAt;
+    }
+
+    /**
+     * Get the identity that last modified the resource.
+     *
+     * @return the identity that last modified the resource.
+     */
+    public String lastModifiedBy() {
+        return this.lastModifiedBy;
+    }
+
+    /**
+     * Get the type of identity that last modified the resource.
+     *
+     * @return the type of identity that last modified the resource.
+     */
+    public CreatedByType lastModifiedByType() {
+        return this.lastModifiedByType;
+    }
+


SystemData seems very generic. Is this name finalized? Can we change this to AzureResourceSystemData?

srnagar · 2021-02-26T09:35:32Z

sdk/core/azure-core-management/src/main/java/com/azure/core/management/SystemData.java

+    public CreatedByType createdByType() {
+        return this.createdByType;
+    }
+
+    /**
+     * Get the timestamp of resource creation (UTC).
+     *
+     * @return the timestamp of resource creation (UTC).
+     */
+    public OffsetDateTime createdAt() {
+        return this.createdAt;
+    }
+
+    /**
+     * Get the identity that last modified the resource.
+     *
+     * @return the identity that last modified the resource.
+     */
+    public String lastModifiedBy() {
+        return this.lastModifiedBy;
+    }
+
+    /**
+     * Get the type of identity that last modified the resource.
+     *
+     * @return the type of identity that last modified the resource.
+     */
+    public CreatedByType lastModifiedByType() {
+        return this.lastModifiedByType;
+    }
+


The method names look fine but just want to check, do we prefix get for getter methods in other arm model types?

srnagar · 2021-02-26T09:40:42Z

sdk/core/azure-core-management/src/main/java/com/azure/core/management/SystemData.java

+import java.time.OffsetDateTime;
+
+/** Metadata pertaining to creation and last modification of the resource. */
+public final class SystemData {


Is this type missing a constructor? How are the property values set?

The class is initialized by Jackson (via reflection), hence no constructor (or just default constructor). All properties is read-only, hence no setter.

weidongxu-microsoft · 2021-03-05T04:16:51Z

So far:

Name of SystemData is not changed
Name of CreatedByType is changed to ResourceAuthorIdentityType

Updated the swagger files for fixing the casing issue (Azure#18303)

support systemData for Azure resource

62de7f7

weidongxu-microsoft added the Mgmt - Track 2 label Dec 22, 2020

ghost added the Azure.Core azure-core label Dec 22, 2020

weidongxu-microsoft requested review from xseeseesee, ChenTanyi and yungezz December 22, 2020 05:26

weidongxu-microsoft added 2 commits December 22, 2020 13:28

nit

fa58baf

more tests

332f548

weidongxu-microsoft marked this pull request as ready for review December 22, 2020 08:51

weidongxu-microsoft requested review from alzimmermsft, anuchandy, hemanttanwar, jianghaolu and srnagar as code owners December 22, 2020 08:51

weidongxu-microsoft commented Dec 22, 2020

View reviewed changes

sdk/core/azure-core-management/src/main/java/com/azure/core/management/CreatedByType.java Outdated Show resolved Hide resolved

Merge branch 'master' into mgmt-core_support-system-data

96ce067

ChenTanyi approved these changes Jan 6, 2021

View reviewed changes

weidongxu-microsoft marked this pull request as draft January 11, 2021 03:24

weidongxu-microsoft added 2 commits February 25, 2021 14:36

Merge branch 'master' into mgmt-core_support-system-data

838f377

use SystemData as stand-along

61b0edf

weidongxu-microsoft removed the request for review from xseeseesee February 25, 2021 07:08

weidongxu-microsoft marked this pull request as ready for review February 25, 2021 07:13

weidongxu-microsoft commented Feb 25, 2021

View reviewed changes

yungezz approved these changes Feb 25, 2021

View reviewed changes

srnagar reviewed Feb 26, 2021

View reviewed changes

weidongxu-microsoft added 5 commits March 1, 2021 10:22

rename CreateByType to ResourceAuthorIdentityType

d6bc4f8

rename CreateByType to ResourceAuthorIdentityType

9ca8b37

add JsonProperty write-only

17aa26e

Merge branch 'master' into mgmt-core_support-system-data

dd5d4be

update test with better data (masked)

ae92627

nit

26d5336

weidongxu-microsoft merged commit f2f60f0 into Azure:master Mar 5, 2021

weidongxu-microsoft deleted the mgmt-core_support-system-data branch March 5, 2021 04:23

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-java that referenced this pull request Mar 23, 2022

CodeGen from PR 18303 in Azure/azure-rest-api-specs

d2d5cea

Updated the swagger files for fixing the casing issue (Azure#18303)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgmt-core, support systemData for Azure resource #18303

mgmt-core, support systemData for Azure resource #18303

weidongxu-microsoft commented Dec 22, 2020 •

edited

Loading

weidongxu-microsoft Feb 25, 2021 •

edited

Loading

weidongxu-microsoft Feb 25, 2021 •

edited

Loading

srnagar Feb 26, 2021

srnagar Feb 26, 2021

srnagar Feb 26, 2021

weidongxu-microsoft Feb 26, 2021 •

edited

Loading

srnagar Feb 26, 2021

srnagar Feb 26, 2021

srnagar Feb 26, 2021

srnagar Feb 26, 2021

weidongxu-microsoft Feb 26, 2021

weidongxu-microsoft commented Mar 5, 2021

mgmt-core, support systemData for Azure resource #18303

mgmt-core, support systemData for Azure resource #18303

Conversation

weidongxu-microsoft commented Dec 22, 2020 • edited Loading

weidongxu-microsoft Feb 25, 2021 • edited Loading

Choose a reason for hiding this comment

weidongxu-microsoft Feb 25, 2021 • 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

weidongxu-microsoft Feb 26, 2021 • 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

weidongxu-microsoft commented Mar 5, 2021

weidongxu-microsoft commented Dec 22, 2020 •

edited

Loading

weidongxu-microsoft Feb 25, 2021 •

edited

Loading

weidongxu-microsoft Feb 25, 2021 •

edited

Loading

weidongxu-microsoft Feb 26, 2021 •

edited

Loading