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

Derivedtype support (part 2) #410

Merged
merged 5 commits into from May 27, 2016

Conversation

Projects
None yet
3 participants
@chinadragon0515
Contributor

chinadragon0515 commented May 26, 2016

Issues

This pull request fixes part of issue #117

Description

Add support for the query

Checklist (Uncheck if it is not completed)

  • [ x ] Test cases added
  • [ x ] Build and test with one-click build and test script passed
@msftclas

This comment has been minimized.

msftclas commented May 26, 2016

Hi @chinadragon0515, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Dashuang He). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

if (sourceType == resultType)
{
return new DerivedDataReference(source);
return new QueryModelReference(source.EntitySet,source.Type);

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

space after comma

This comment has been minimized.

@chinadragon0515
@@ -174,6 +174,9 @@ private QueryModelReference ComputeModelReference()
QueryModelReference modelReference = null;
var methodCall = this.VisitedNode as MethodCallExpression;
var parameter = this.VisitedNode as ParameterExpression;
var member = this.VisitedNode as MemberExpression;

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

Personally I feel the previous pattern is more efficient...You don't need to perform three casts at first.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

Two options,

  1. like what I do now.
  2. Any if condition is not null, return
    Which one you prefer?

Previous logic does not return, it will go through all logic and last return.

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 29, 2016

Contributor

Thanks for the info. I prefer option 2. Better check each condition then return if fail.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 30, 2016

Contributor

OK, updated.

var collectionType = modelReference.Type as IEdmCollectionType;
if (collectionType != null)
{
modelReference = new QueryModelReference(modelReference.EntitySet, collectionType.ElementType.Definition);

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

avoid deep nest and break this line. It's too long. StyleCop will catch this line.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

Updated.

private IEdmEntitySet edmEntitySet;
private IEdmType edmType;
internal QueryModelReference()

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

Is the default ctor useful? If not, please consider remove it.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

It is used by sub class.Cannot be removed.

{
private IEdmEntitySet edmEntitySet;

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

Set these two fields readonly since they will not be changed after ctor.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

Updated

/// <param name="propertyName">
/// The name of a property.
/// </param>
public PropertyModelReference(QueryModelReference source, string propertyName)

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

Mark the ctor internal: 1) keep consistent; 2) we don't want user to be able to instantiate this class.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

Updated

private void HandleCastPathSegment(ODataPathSegment segment)
{
var castSegment = (CastPathSegment)segment;
Type elementType = GetClrType(castSegment.CastType);

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

uniformly use var. line 319, 327, 329

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

updated.

{
if (namespaceName == null)
{
mapper.TryGetRelevantType(api.Context, name, out elementType);

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

The name is NOT the type name but the resource name or operation name. So model mapper cannot be called like this.

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

In next PR, this is already updated.

@@ -63,7 +63,7 @@
</providers>
</entityFramework>
<connectionStrings>
<add name="TrippinModel" connectionString="data source=(localdb)\MSSQLLocalDB;initial catalog=TRIPPINDB;integrated security=True;connect timeout=30;MultipleActiveResultSets=True;App=EntityFramework" providerName="System.Data.SqlClient"/>
<add name="TrippinModel" connectionString="data source=(localdb)\MSSQLLocalDB;initial catalog=TRIPPINE2EDB;integrated security=True;connect timeout=30;MultipleActiveResultSets=True;App=EntityFramework" providerName="System.Data.SqlClient"/>

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

Did you also change the db name in Trippin DbContext ctor?

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

No code refer to this name, code only refer to the name which is TrippinModel, change the DB name as I created TripPin sample which use same DB, want them to run separately.

@@ -235,7 +235,7 @@ public void UQProperty()
this.TestClientContext.AddToPeople(person);
this.TestClientContext.SaveChanges();
int personId = person.PersonId;
long? personId = person.PersonId;

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

why change the Id type? Is it related to type cast or derived type support?

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

Not related to derived type, before we change ID to long to cover an issue ID is long does not work. And proxy file is not updated.
This time, I need to update proxy file to include derived type, the ID is changed.

/// </summary>
/// <param name="context">
/// A query context.
/// </param>
/// <param name="name">
/// The name of an entity set, singleton or function import.
/// </param>
public DataSourceStubReference(QueryContext context, string name)
public DataSourceStubModelReference(QueryContext context, string name)

This comment has been minimized.

@lewischeng-ms

lewischeng-ms May 27, 2016

Contributor

Mark as internal

This comment has been minimized.

@chinadragon0515

chinadragon0515 May 27, 2016

Contributor

Updated

@chinadragon0515

This comment has been minimized.

Contributor

chinadragon0515 commented May 27, 2016

All comments are addressed in next PR, will merge this PR first, and more comments, continue add here, I will address in next PR.

@chinadragon0515 chinadragon0515 merged commit c69ac00 into OData:master May 27, 2016

@chinadragon0515 chinadragon0515 deleted the chinadragon0515:derivedtype branch May 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment