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

Allow multiple relationships in ConnectorType #2747

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
30 changes: 14 additions & 16 deletions src/libraries/Microsoft.PowerFx.Connectors/OpenApiExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
using Microsoft.PowerFx.Core.Utils;
using Microsoft.PowerFx.Types;
using static Microsoft.PowerFx.Connectors.Constants;

#pragma warning disable SA1000 // The keyword 'new' should be followeed by a space
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we just want to remove this at the editorconfig level?


namespace Microsoft.PowerFx.Connectors
{
Expand Down Expand Up @@ -325,7 +327,7 @@ internal static bool TryGetOpenApiValue(IOpenApiAny openApiAny, FormulaType form
}
else if (openApiAny is IDictionary<string, IOpenApiAny> o)
{
Dictionary<string, FormulaValue> dvParams = new ();
Dictionary<string, FormulaValue> dvParams = new();

foreach (KeyValuePair<string, IOpenApiAny> kvp in o)
{
Expand Down Expand Up @@ -676,17 +678,13 @@ internal static ConnectorType GetConnectorType(this ISwaggerParameter openApiPar
return new ConnectorType(schema, openApiParameter, FormulaType.String, hiddenfields.ToRecordType());
}

//ConnectorType propertyType = new OpenApiParameter() { Name = propLogicalName, Required = schema.Required.Contains(propLogicalName), Schema = kv.Value, Extensions = kv.Value.Extensions }.GetConnectorType(settings.Stack(schemaIdentifier));
ConnectorType propertyType = new SwaggerParameter(propLogicalName, schema.Required.Contains(propLogicalName), kv.Value, kv.Value.Extensions).GetConnectorType(settings.Stack(schemaIdentifier));

if (settings.SqlRelationships != null)
{
SqlRelationship relationship = settings.SqlRelationships.FirstOrDefault(sr => sr.ColumnName == propLogicalName);
IEnumerable<SqlRelationship> relationships = settings.SqlRelationships?.Where(sr => sr.ColumnName == propLogicalName);

if (relationship != null)
{
propertyType.SetRelationship(relationship);
}
if (relationships?.Any() == true)
{
propertyType.SetRelationships(relationships);
}

settings.UnStack();
Expand Down Expand Up @@ -818,7 +816,7 @@ public static HttpMethod ToHttpMethod(this OperationType key)
}

public static FormulaType GetReturnType(this OpenApiOperation openApiOperation, ConnectorCompatibility compatibility)
{
{
ConnectorType connectorType = openApiOperation.GetConnectorReturnType(compatibility);
FormulaType ft = connectorType.HasErrors ? ConnectorType.DefaultType : connectorType?.FormulaType ?? new BlankType();
return ft;
Expand Down Expand Up @@ -890,7 +888,7 @@ internal static ConnectorType GetConnectorReturnType(this OpenApiOperation openA
/// <exception cref="NotImplementedException">When we cannot determine the content type to use.</exception>
public static (string ContentType, OpenApiMediaType MediaType) GetContentTypeAndSchema(this IDictionary<string, OpenApiMediaType> content)
{
Dictionary<string, OpenApiMediaType> list = new ();
Dictionary<string, OpenApiMediaType> list = new();

foreach (var ct in _knownContentTypes)
{
Expand Down Expand Up @@ -1045,7 +1043,7 @@ internal static ConnectorDynamicValue GetDynamicValue(this ISwaggerExtensions pa
{
// Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
IDictionary<string, IOpenApiAny> op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is IDictionary<string, IOpenApiAny> apiString ? apiString : null;
ConnectorDynamicValue cdv = new (op_prms);
ConnectorDynamicValue cdv = new(op_prms);

// Mandatory operationId for connectors, except when capibility or builtInOperation are defined
apiObj.WhenPresent("operationId", (opId) => cdv.OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId));
Expand Down Expand Up @@ -1074,7 +1072,7 @@ internal static ConnectorDynamicList GetDynamicList(this ISwaggerExtensions para
{
// Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
IDictionary<string, IOpenApiAny> op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is IDictionary<string, IOpenApiAny> apiString ? apiString : null;
ConnectorDynamicList cdl = new (op_prms)
ConnectorDynamicList cdl = new(op_prms)
{
OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),
};
Expand Down Expand Up @@ -1107,7 +1105,7 @@ internal static ConnectorDynamicList GetDynamicList(this ISwaggerExtensions para

internal static Dictionary<string, IConnectorExtensionValue> GetParameterMap(this IDictionary<string, IOpenApiAny> opPrms, SupportsConnectorErrors errors, bool forceString = false)
{
Dictionary<string, IConnectorExtensionValue> dvParams = new ();
Dictionary<string, IConnectorExtensionValue> dvParams = new();

if (opPrms == null)
{
Expand Down Expand Up @@ -1204,7 +1202,7 @@ internal static ConnectorDynamicSchema GetDynamicSchema(this ISwaggerExtensions
// Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
IDictionary<string, IOpenApiAny> op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is IDictionary<string, IOpenApiAny> apiString ? apiString : null;

ConnectorDynamicSchema cds = new (op_prms)
ConnectorDynamicSchema cds = new(op_prms)
{
OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),
};
Expand Down Expand Up @@ -1236,7 +1234,7 @@ internal static ConnectorDynamicProperty GetDynamicProperty(this ISwaggerExtensi
// Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
IDictionary<string, IOpenApiAny> op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is IDictionary<string, IOpenApiAny> apiString ? apiString : null;

ConnectorDynamicProperty cdp = new (op_prms)
ConnectorDynamicProperty cdp = new(op_prms)
{
OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

using System.Collections.Generic;
using System.Linq;
using Microsoft.PowerFx.Types;

namespace Microsoft.PowerFx.Connectors
{
public static class CdpExtensions
{
public static bool TryGetFieldExternalTableName(this RecordType recordType, string fieldName, out string tableName, out string foreignKey)
public static bool TryGetFieldRelationships(this RecordType recordType, string fieldName, out IEnumerable<ConnectorRelationship> relationships)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TryGetFieldRelationships

I'll add [Obsolete]

{
if (recordType is not CdpRecordType cdpRecordType)
if (recordType is not CdpRecordType cdpRecordType || (!cdpRecordType.TryGetFieldRelationships(fieldName, out relationships) && relationships.Any()))
{
tableName = null;
foreignKey = null;
relationships = null;
return false;
}
}

return cdpRecordType.TryGetFieldExternalTableName(fieldName, out tableName, out foreignKey);
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,17 @@ internal CdpRecordType(ConnectorType connectorType, ICdpTableResolver tableResol
TableResolver = tableResolver;
}

public bool TryGetFieldExternalTableName(string fieldName, out string tableName, out string foreignKey)
public bool TryGetFieldRelationships(string fieldName, out IEnumerable<ConnectorRelationship> relationships)
{
tableName = null;
foreignKey = null;

ConnectorType connectorType = ConnectorType.Fields.First(ct => ct.Name == fieldName);

if (connectorType == null || connectorType.ExternalTables?.Any() != true)
{
relationships = null;
return false;
}

tableName = connectorType.ExternalTables.First();
foreignKey = connectorType.ForeignKey;
relationships = connectorType.ExternalTables;
return true;
}

Expand All @@ -60,7 +57,8 @@ private bool TryGetFieldType(string fieldName, bool ignorelationship, out Formul
return true;
}

string tableName = field.ExternalTables.First();
// $$$ We should NOT call First() here but consider all possible foreign tables
string tableName = field.ExternalTables.First().ForeignTable;

try
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

using Microsoft.PowerFx.Core.Utils;

namespace Microsoft.PowerFx.Connectors
{
public class ConnectorRelationship
Copy link
Contributor

Choose a reason for hiding this comment

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

ConnectorRelationship

Is this CDP specific?

If so, how do non-CDP shims (ServiceNow) leverage it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now, yes
as discussed this is a temporary thing
I'll add [Obsolete] to it

{
public string ForeignTable { get; internal init; }

public string ForeignKey { get; internal init; }

public override int GetHashCode()
{
return Hashing.CombineHash(ForeignTable?.GetHashCode() ?? 0, ForeignKey?.GetHashCode() ?? 0);
}

public override string ToString() => $"{ForeignTable ?? "<null>"}:{ForeignKey ?? "<null>"}";
}
}
119 changes: 106 additions & 13 deletions src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace Microsoft.PowerFx.Connectors
// FormulaType is used to represent the type of the parameter in the Power Fx expression as used in Power Apps
// ConnectorType contains more details information coming from the swagger file and extensions
[DebuggerDisplay("{FormulaType._type}")]
public class ConnectorType : SupportsConnectorErrors
public class ConnectorType : SupportsConnectorErrors, IEquatable<ConnectorType>
LucGenetier marked this conversation as resolved.
Show resolved Hide resolved
{
// "name"
public string Name { get; internal set; }
Expand Down Expand Up @@ -112,11 +112,9 @@ public class ConnectorType : SupportsConnectorErrors
internal ISwaggerSchema Schema { get; private set; } = null;

// Relationships to external tables
internal List<string> ExternalTables { get; set; }
internal IEnumerable<ConnectorRelationship> ExternalTables => _externalTables;

internal string RelationshipName { get; set; }
LucGenetier marked this conversation as resolved.
Show resolved Hide resolved

internal string ForeignKey { get; set; }
private List<ConnectorRelationship> _externalTables;

internal ConnectorType(ISwaggerSchema schema, ISwaggerParameter openApiParameter, FormulaType formulaType, ErrorResourceKey warning = default)
{
Expand Down Expand Up @@ -148,9 +146,11 @@ internal ConnectorType(ISwaggerSchema schema, ISwaggerParameter openApiParameter
// SalesForce only
if (schema.ReferenceTo != null && schema.ReferenceTo.Count == 1)
{
ExternalTables = new List<string>(schema.ReferenceTo);
RelationshipName = schema.RelationshipName;
ForeignKey = null; // SalesForce doesn't provide it, defaults to "Id"
_externalTables = schema.ReferenceTo.Select(r => new ConnectorRelationship()
{
ForeignTable = r,
ForeignKey = null // Seems to always be Id
}).ToList();
}

Fields = Array.Empty<ConnectorType>();
Expand Down Expand Up @@ -290,12 +290,18 @@ internal DisplayNameProvider DisplayNameProvider

private DisplayNameProvider _displayNameProvider;

internal void SetRelationship(SqlRelationship relationship)
internal void SetRelationships(IEnumerable<SqlRelationship> relationships)
{
ExternalTables ??= new List<string>();
ExternalTables.Add(relationship.ReferencedTable);
RelationshipName = relationship.RelationshipName;
ForeignKey = relationship.ReferencedColumnName;
_externalTables ??= new List<ConnectorRelationship>();

foreach (SqlRelationship relationship in relationships)
{
_externalTables.Add(new ConnectorRelationship()
{
ForeignTable = relationship.ReferencedTable,
ForeignKey = relationship.ReferencedColumnName
});
}
}

private void AggregateErrors(ConnectorType[] types)
Expand Down Expand Up @@ -336,5 +342,92 @@ private Dictionary<string, FormulaValue> GetEnum()

return enumDisplayNames.Zip(enumValues, (dn, ev) => new KeyValuePair<string, FormulaValue>(dn, ev)).ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
}

public bool Equals(ConnectorType other)
{
if (other == null)
{
return false;
}

return this.Name == other.Name &&
LucGenetier marked this conversation as resolved.
Show resolved Hide resolved
this.DisplayName == other.DisplayName &&
this.Description == other.Description &&
this.IsRequired == other.IsRequired &&
Enumerable.SequenceEqual((IList<ConnectorType>)this.Fields ?? Array.Empty<ConnectorType>(), (IList<ConnectorType>)other.Fields ?? Array.Empty<ConnectorType>()) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

(IList)this.Fields

This is recursive - how do we verify this is not a stack overflow?

Enumerable.SequenceEqual((IList<ConnectorType>)this.HiddenFields ?? Array.Empty<ConnectorType>(), (IList<ConnectorType>)other.HiddenFields ?? Array.Empty<ConnectorType>()) &&
this.ExplicitInput == other.ExplicitInput &&
this.IsEnum == other.IsEnum &&
Enumerable.SequenceEqual((IList<FormulaValue>)this.EnumValues ?? Array.Empty<FormulaValue>(), (IList<FormulaValue>)other.EnumValues ?? Array.Empty<FormulaValue>()) &&
Enumerable.SequenceEqual((IList<string>)this.EnumDisplayNames ?? Array.Empty<string>(), (IList<string>)other.EnumDisplayNames ?? Array.Empty<string>()) &&
this.Visibility == other.Visibility &&
((this.Capabilities == null && other.Capabilities == null) || this.Capabilities.Equals(other.Capabilities)) &&
this.KeyType == other.KeyType &&
this.KeyOrder == other.KeyOrder &&
this.Permission == other.Permission &&
this.NotificationUrl == other.NotificationUrl &&
((this.HiddenRecordType == null && other.HiddenRecordType == null) || this.HiddenRecordType.Equals(other.HiddenRecordType)) &&
this.Binary == other.Binary &&
this.MediaKind == other.MediaKind &&
Enumerable.SequenceEqual((IList<string>)this.ExternalTables ?? Array.Empty<string>(), (IList<string>)other.ExternalTables ?? Array.Empty<string>());
}

public override bool Equals(object obj)
{
return Equals(obj as ConnectorType);
}

public override int GetHashCode()
{
int h = Hashing.CombineHash(Name.GetHashCode(), DisplayName?.GetHashCode() ?? 0, Description?.GetHashCode() ?? 0, IsRequired.GetHashCode());

if (Fields != null)
{
foreach (ConnectorType field in Fields)
{
h = Hashing.CombineHash(h, field.GetHashCode());
}
}

if (HiddenFields != null)
{
foreach (ConnectorType hiddenField in HiddenFields)
{
h = Hashing.CombineHash(h, hiddenField.GetHashCode());
}
}

h = Hashing.CombineHash(h, ExplicitInput.GetHashCode(), IsEnum.GetHashCode(), Visibility.GetHashCode());

if (EnumValues != null)
{
foreach (FormulaValue enumValue in EnumValues)
{
h = Hashing.CombineHash(h, enumValue.GetHashCode());
}
}

if (EnumDisplayNames != null)
{
foreach (string enumDisplayName in EnumDisplayNames)
{
h = Hashing.CombineHash(h, enumDisplayName.GetHashCode());
}
}

h = Hashing.CombineHash(h, Capabilities.GetHashCode(), KeyType.GetHashCode(), KeyOrder.GetHashCode(), Permission.GetHashCode(), NotificationUrl?.GetHashCode() ?? 0, HiddenRecordType.GetHashCode());
h = Hashing.CombineHash(h, Binary.GetHashCode());
h = Hashing.CombineHash(h, MediaKind.GetHashCode());

if (ExternalTables != null)
{
foreach (ConnectorRelationship relationship in ExternalTables)
{
h = Hashing.CombineHash(h, relationship.GetHashCode());
}
}

return h;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.Json.Serialization;
Expand All @@ -12,7 +13,7 @@

namespace Microsoft.PowerFx.Connectors
{
internal sealed class ColumnCapabilities : ColumnCapabilitiesBase, IColumnsCapabilities
internal sealed class ColumnCapabilities : ColumnCapabilitiesBase, IColumnsCapabilities, IEquatable<ColumnCapabilities>
{
[JsonInclude]
[JsonPropertyName(CapabilityConstants.ColumnProperties)]
Expand Down Expand Up @@ -72,5 +73,32 @@ public static ColumnCapabilities ParseColumnCapabilities(IDictionary<string, IOp
IsChoice = isChoice
});
}

public bool Equals(ColumnCapabilities other)
Copy link
Contributor

Choose a reason for hiding this comment

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

public bool Equals(ColumnCapabilities other)

Intrinsically, why would we ever need to compare ColumnCapabilities?

{
return Enumerable.SequenceEqual((IEnumerable<KeyValuePair<string, ColumnCapabilitiesBase>>)this._childColumnsCapabilities ?? Array.Empty<KeyValuePair<string, ColumnCapabilitiesBase>>(), (IEnumerable<KeyValuePair<string, ColumnCapabilitiesBase>>)other._childColumnsCapabilities ?? Array.Empty<KeyValuePair<string, ColumnCapabilitiesBase>>()) &&
this.Capabilities.Equals(other.Capabilities);
}

public override bool Equals(object obj)
{
return Equals(obj as ColumnCapabilities);
}

public override int GetHashCode()
{
int h = Capabilities.GetHashCode();

if (_childColumnsCapabilities != null)
{
foreach (KeyValuePair<string, ColumnCapabilitiesBase> kvp in _childColumnsCapabilities)
{
h = Hashing.CombineHash(h, kvp.Key.GetHashCode());
h = Hashing.CombineHash(h, kvp.Value.GetHashCode());
}
}

return h;
}
}
}
Loading