commonids: add composite resource id

[catriona-m]

No description provided.

[tombuildsstuff]

hey @catriona-m

Thanks for this PR - apologies for the delayed review here.

Taking a look through on the whole this is looking pretty good, if we can fix up the comments then this should otherwise be good to go 👍

Thanks!

[tombuildsstuff]

could we output the parsed id part as a part of the error to make this clearer?

Suggested change

	return nil, fmt.Errorf("parsing first id of CompositeResourceID: %v", err)
	}
	err = output.First.FromParseResult(*firstParseResult)
	if err != nil {
	return nil, fmt.Errorf("populating first id of CompositeResourceID: %v", err)
	}
	// Parse the second of the two Resource IDs from the components
	secondParser := resourceids.NewParserFromResourceIdType(output.Second)
	secondParseResult, err := secondParser.Parse(components[1], insensitively)
	if err != nil {
	return nil, fmt.Errorf("parsing second id of CompositeResourceID: %v", err)
	}
	err = output.Second.FromParseResult(*secondParseResult)
	if err != nil {
	return nil, fmt.Errorf("populating second id of CompositeResourceID: %v", err)
	return nil, fmt.Errorf("parsing first id part %q of CompositeResourceID: %v", components[0], err)
	}
	err = output.First.FromParseResult(*firstParseResult)
	if err != nil {
	return nil, fmt.Errorf("populating first id part %q of CompositeResourceID: %v", components[0], err)
	}
	// Parse the second of the two Resource IDs from the components
	secondParser := resourceids.NewParserFromResourceIdType(output.Second)
	secondParseResult, err := secondParser.Parse(components[1], insensitively)
	if err != nil {
	return nil, fmt.Errorf("parsing second id part %q of CompositeResourceID: %v", components[1], err)
	}
	err = output.Second.FromParseResult(*secondParseResult)
	if err != nil {
	return nil, fmt.Errorf("populating second id part %q of CompositeResourceID: %v", components[1], err)

[tombuildsstuff]

given we're providing wrapper functions above, does this method need to be public?

[tombuildsstuff]

could we document this struct/the functions to explain how this is intended to be used?

[tombuildsstuff]

Left a few minor comments inline but otherwise 👍

[tombuildsstuff]

Can we call out that this is a Composite Resource ID:

Suggested change

	// String returns a human-readable description of this Composite Resource Id
	func (id CompositeResourceID[T1, T2]) String() string {
	fmtString := "%s\n%s"
	return fmt.Sprintf(fmtString, id.First.String(), id.Second.String())
	}
	// String returns a human-readable description of this Composite Resource Id
	func (id CompositeResourceID[T1, T2]) String() string {
	fmtString := "Composite Resource ID (%s | %s)"
	return fmt.Sprintf(fmtString, id.First.String(), id.Second.String())
	}

[tombuildsstuff]

minor but could we update parse to be parseCompositeResourceID so that this isn't used unintentionally?

Suggested change

	return parse(input, first, second, false)
	}
	// ParseCompositeResourceIDInsensitively parses 'input' and two ResourceIds (first,second) case-insensitively into a CompositeResourceID
	// note: this method should only be used for API response data and not user input
	func ParseCompositeResourceIDInsensitively[T1 resourceids.ResourceId, T2 resourceids.ResourceId](input string, first T1, second T2) (*CompositeResourceID[T1, T2], error) {
	return parse(input, first, second, true)
	}
	func parse[T1 resourceids.ResourceId, T2 resourceids.ResourceId](input string, first T1, second T2, insensitively bool) (*CompositeResourceID[T1, T2], error) {
	return parseCompositeResourceID(input, first, second, false)
	}
	// ParseCompositeResourceIDInsensitively parses 'input' and two ResourceIds (first,second) case-insensitively into a CompositeResourceID
	// note: this method should only be used for API response data and not user input
	func ParseCompositeResourceIDInsensitively[T1 resourceids.ResourceId, T2 resourceids.ResourceId](input string, first T1, second T2) (*CompositeResourceID[T1, T2], error) {
	return parseCompositeResourceID(input, first, second, true)
	}
	func parseCompositeResourceID[T1 resourceids.ResourceId, T2 resourceids.ResourceId](input string, first T1, second T2, insensitively bool) (*CompositeResourceID[T1, T2], error) {

[tombuildsstuff]

minor but could we document these fields so that it's clearer what these are?

Suggested change

	First T1
	Second T2
	// First specifies the first component of this Resource ID.
	// This is in the format `{first}|{second}`.
	First T1
	// Second specifies the second component of this Resource ID
	// This is in the format `{first}|{second}`.
	Second T2

[tombuildsstuff]

One really minor nit for readability purposes but this otherwise LGTM 👍

[wuxu92]

Is there a plan to release this PR and update it in the AzureRM dependency?