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

Deep load features and sql dump #471

Merged
merged 3 commits into from Mar 4, 2019

Conversation

Projects
None yet
3 participants
@jeremeguenther
Copy link
Collaborator

jeremeguenther commented Jan 4, 2019

Misc fixes. Plus Added new deep load features, and the ability to dump the basic sql schema out to files for source control inclusion. Also added the ability to re-execute the views against the server to refresh their meta data.

Deep load features and sql dump
Misc fixes.  Plus Added new deep load features, and the ability to dump the basic sql schema out to files for source control inclusion.  Also added the ability to re-execute the views against the server to refresh their meta data.

@jeremeguenther jeremeguenther requested a review from niemyjski Jan 4, 2019

/// <returns>Returns a typed collection of Entity objects.</returns>
public virtual <%=listName%><Entity> <%= MethodNames.Find %>(TransactionManager transactionManager, IFilterParameterCollection parameters, string orderBy, int start, int pageLength, out int count)
{
<% // TODO: make this method abstract after it has been fully implemented! %>

This comment has been minimized.

@niemyjski

niemyjski Jan 4, 2019

Contributor

Was this meant to be implemented?

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 4, 2019

Author Collaborator

Yes, this is the supporting class for deep loads. My changes here, assign from converting tabs to spaces, are in support of the new deep load features, adding the Flag attribute to the Enum, and granting visibility of the load depth to the event triggered.

@@ -3091,7 +3341,40 @@
#region Data Access project
if (GenerateDataAccessLayer) // && !File.Exists(rootPathDAL + "\\" + NameSpace + ".DataAccessLayer.csproj"))
{
#region Data Access Layer base
#region SQL Export Files

This comment has been minimized.

@niemyjski

niemyjski Jan 4, 2019

Contributor

I'm curious why you'd want a separate project for just the sql files. Also what is preventing no longer valid sql procs from being extracted and now in your source control (e.g., You had a table, you generated netTiers against it it and it created a bunch. We change .netTiers to use a newly named proc (probably would never happen) and those existing procs are now extracted? It might be nice to put all the ones that we know we didn't create with a comment or into a custom folder at some point.

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 4, 2019

Author Collaborator

The separate project makes it much easier for older versions of visual studio to detect the changes and include the newly generated files in source control. I've heard that newer visual studio projects are folder based, but I have not experienced that yet. The sprocs I am generating are only the custom sprocs, I am not dumping out any of the auto generated ones. For your scenario to happen, if we changed .netTiers to use a different prefix so it generated a whole new set of sprocs, then the "custom sproc prefix" setting would have to be set to what .netTiers used to generate in order for this to pick them up. However, cleaning up old unused .netTiers auto generated sprocs has always been an issue (e.g. when a table is deleted or renamed); I have not come up with a good solution for that one; this also feeds into issues of left over code files in source control when a table is deleted or renamed. Did I understand the concern correctly?

<%@ Import Namespace="System.Text" %>
<%@ Import Namespace="System.Text.RegularExpressions" %>
<%@ Import Namespace="System.Collections" %>
<%=(CommandSchema.CommandText) %>

This comment has been minimized.

@niemyjski

niemyjski Jan 4, 2019

Contributor

I'm not sure how reliable this is when it comes to table/clr functions and complex sql functions. We through a lot of extra metadata into extended data about what kind of function it is. I can't remember if this is always populated.

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 4, 2019

Author Collaborator

This file is specific to stored procedures. However, I am assuming you are referring to all the new files in this folder. Currently I am not even attempting to extract sql functions, that would be a nice feature to have though. The table extraction is definitely not perfect, it is missing constraints and defaults at a minimum. The Views extraction is decent, but again, it is missing a couple of pieces like setting ansi_nulls, so I am pulling those from the standard .netTiers is using for the sprocs. The Sproc extraction itself seems to work the best, it just dumps the entire text of the procedure to the file. What I have is enough for source control at this point, but I am hoping to improve it at some point in the future.

This comment has been minimized.

@niemyjski

niemyjski Jan 4, 2019

Contributor

It's better than nothing :)

return string.Format("\"{0}\", typeof({1}), System.Data.{2}, ",
string defaultValue = GetCSDefaultValueByType(column, true);

return string.Format("\"{0}\", typeof({1}), System.Data.{2}, /*precision*/ {3}, /*scale*/ {4}, /*default*/ {5}, ",

This comment has been minimized.

@niemyjski

niemyjski Jan 4, 2019

Contributor

Changes in this file scare me a little bit. I'm not 100% sure where this method call is being consumed by, but I remember the nightmares to add oracle support and it was very touchy and I know the precisions and scales weren't always right or expected when you are from a SQL Server world.

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 4, 2019

Author Collaborator

Currently the biggest changes in this file are in support of the new attributes added to the Column Enum. Those should not be touching any actual functionality, just presenting users with more data about the database if it exists. The other two changes were submitted by other users years ago and looked common sense, so I included them. It would be nice if I could setup some sort of environment to generate against several different db types for final testing.

@niemyjski
Copy link
Contributor

niemyjski left a comment

It looks good to me, we may want to generate against a non sql database to ensure everything still compiles and runs. I Left some comments

<%@ Property Name="IncludeSqlSprocs" Type="System.Boolean" Default="False" Category="Option" Description="" %>
<%@ Property Name="IncludeSqlViews" Type="System.Boolean" Default="False" Category="Option" Description="" %>
<%@ Property Name="IncludeSqlTables" Type="System.Boolean" Default="False" Category="Option" Description="" %>

This comment has been minimized.

@jmhinnen

jmhinnen Jan 4, 2019

I assume the options here are for the SQL dump functionality. You might include a description as to what specifically this option is for. Maybe something like: "Dump Store Procedures to Files". IncludeSqlProcs is a little bit vague.

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 4, 2019

Author Collaborator

Ah, now I see why I did not add the descriptions there. Because those are sub templates. I always wondered why you guys put so many descriptions in the sub templates. Was it common for people to try and run the templates individually without the master template?

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 5, 2019

Author Collaborator

I added descriptions

@jmhinnen

This comment has been minimized.

Copy link

jmhinnen commented Jan 4, 2019

In general I can't really complain about the features you put in. I am not a huge fan of deep load so I don't have much to say on the changes to that. I used to tell people that if it doesn't work for them, they should code something specific for the case because generally speaking, deep load is not a very good performing design overall on how to load child data unless you are only loading a single parent object. I never really spent time trying to make it work for all cases. It just seemed a bit out of scope in my mind. But, that's just my opinion and if you found a way to cover more use cases, then I think that's great.

I cringed a little on the tabs to spaces but that's a topic way out of scope lol. Nice job on the changes!

/// <returns>A <see cref="<%=returnTypeForComment%>"/> instance.</returns><%}%>
public <%=returnType%> <%=methodName%>(int start, int pageLength<%=TransformStoredProcedureInputsToMethod(true, command.InputParameters) + TransformStoredProcedureOutputsToMethod(true, command.AllOutputParameters)%>)
{
<%= returnKeyword %> <%=methodName%>(null, start, pageLength <%=TransformStoredProcedur

This comment has been minimized.

@jmhinnen

jmhinnen Jan 4, 2019

Sorry, forgot one comment. you added a new parameter right? Shouldn't there be a default valid for this so existing code does not break? I would think you would default that to -1 or make it a nullable int and make it null. That way existing code that is using it doesn't have compile errors out of the box.

This comment has been minimized.

@jeremeguenther

jeremeguenther Jan 5, 2019

Author Collaborator

I added two new overloads. The oldest supported version of .net does not have default values, so I could not use those.
I also moved one of the deep load methods down to be in order with the rest of them, for some reason it was the only one out of order.

@jeremeguenther

This comment has been minimized.

Copy link
Collaborator Author

jeremeguenther commented Jan 4, 2019

Those last two ideas of yours are great, I need to do those. I don't think I've seen a scenario where the missing default would break it, but better to be safe.

Really? you are a tab guy? I used to be one as well. Most of the world seems to be converting to spaces, and that is the default for visual studio, so eventually I got on board with that. For .netTiers there was no standard, it was a mix of both which drove me crazy, so I just started trying to standardize it all to the Visual Studio default of spaces.

@jeremeguenther

This comment has been minimized.

Copy link
Collaborator Author

jeremeguenther commented Jan 4, 2019

I really like the deep load feature, it is not good for large amounts of data, but it is a great way to include a couple of individual objects that you need really quickly. Typically in business rule type scenarios. Whenever I need flat speed for a page I use a view instead, which is why I want to be able to auto generate methods on views.

@jmhinnen

This comment has been minimized.

Copy link

jmhinnen commented Jan 4, 2019

I am without opinion on the tab vs space conversation. I tell people whatever the default in VS is the kind of guy I am lol. Just cringed at the diff in the PR. It's hard to see what were your changes or what was changing because of the spaces.

And yes, I think it should be consistent!

And I still can't stand deep load lol. It was the buggiest / hardest to get right and never really felt it was 100%. It always felt like something you could never get to 100% based on how netTiers was designed. For example, EF handles deep loading through modifying queries at runtime, netTiers doesn't really do SQL manipulation in real time so it could never be great at it. When we did a POC of a next version of nettiers, we did it more inline with how EF would eventually do it. EF now does it much better than our POC ever did.

Anyway, glad you are getting some use out of it!

@jeremeguenther

This comment has been minimized.

Copy link
Collaborator Author

jeremeguenther commented Jan 4, 2019

I can see where deep load was hard to write, it is pretty complicated. And yes, my ignorance if github is why I submitted the tab conversion at the same time, I am working on getting better at that.

I've tried EF, and I really hate it. I would take .netTiers any day. .netTiers has more bugs in it than EF for sure, but the code is just so much more straight forward, and the queries are nice. My biggest problem with all the DALs that generate their own SQL is how poorly they do it. The only time I have ever seen EF shine is for small projects where you want a DAL really fast and don't care how good the code is or how well it handles the database.

EF doesn't even really feel like a DAL to me, more like a database management system. .netTiers feels like a true more pure DAL to me. You guys really did an awesome job on it.

jeremeguenther added some commits Jan 5, 2019

Deep load features and sql dump
Misc fixes. Plus Added new deep load features, and the ability to dump the basic sql schema out to files for source control inclusion. Also added the ability to re-execute the views against the server to refresh their meta data.
Allow attach debugger
Add commented line for attaching a debugger.
Also fix default decimal and float values.
@jeremeguenther

This comment has been minimized.

Copy link
Collaborator Author

jeremeguenther commented Mar 4, 2019

It looks good to me, we may want to generate against a non sql database to ensure everything still compiles and runs. I Left some comments

I successfully ran against a MySql database. Going to merge in.

@jeremeguenther jeremeguenther merged commit 00b8e4b into netTiers:master Mar 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.