ErikEJ / EFCorePowerTools

Entity Framework Core Power Tools - reverse engineering, migrations and model visualization in Visual Studio & CLI
MIT License
2.21k stars 299 forks source link

Reverse Engeneered Stored Procedure / Function missing StringLength attributes: #2685

Closed BillDuffy closed 3 hours ago

BillDuffy commented 1 day ago

Lack of the StringLength attribute may present problems when code uses that to limit string lengths prior to sending them to the database.

I believe this will suffice.

CREATE TABLE [dbo].[client] (
    [client_id]                         INT             IDENTITY (100, 1) NOT NULL,
    [is_uniformat_level_auto_update]    BIT,
    [defaultCatalog_id]                 INT             NULL,
);
CREATE TABLE [dbo].[uniformat_level] (
    [uniformat_level_id]                            INT             IDENTITY (100, 1) NOT NULL,
    [code]                                          NVARCHAR (50)   NOT NULL,
    [name]                                          NVARCHAR (250)  NOT NULL,
    [Comment]                                       NVARCHAR (100)  NULL,
    [client_id]                                     INT             NOT NULL,
);
CREATE FUNCTION [dbo].[itvf_ClientCostCatalog_TEST] (@Client_ID INT)
RETURNS TABLE 
AS
RETURN 
(
    WITH
    CatalogType AS
    (   SELECT 
            is_uniformat_level_auto_update, -- 0: Full Custom / 1: Default or Hybrid
            defaultCatalog_id               -- If NULL or 0, no default catalog (so a full custom catalog) 
        FROM client
        WHERE client_id = @Client_ID ) 

    -- A) Client-specific Full custom catalog (NO defaults):
    SELECT UL.[code]
            , UL.[uniformat_level_id] 
            , UL.[client_id]
            , UL.[name]
            , UL.Comment
            , 1 as 'RowNbr'
        FROM uniformat_level AS UL
        WHERE (SELECT is_uniformat_level_auto_update from CatalogType) =0  
                AND UL.client_id = @Client_id --AND UL.a.ctive =1   

    UNION ALL  -- While using UNION ALL, only 1 Select query is designed to return results

        -- B) Default Catalog usage only or a Hybrid Catalog
    SELECT * 
        FROM (
            SELECT UL.[code]
            , UL.[uniformat_level_id] 
            , UL.[client_id]
            , UL.[name]
            , UL.Comment
            , row_number() OVER (PARTITION BY CODE ORDER BY CODE ASC, UL.client_id DESC) as 'RowNbr'
            FROM [dbo].[uniformat_level] AS UL
            WHERE (SELECT is_uniformat_level_auto_update from CatalogType) =1  
                AND (UL.client_id = @Client_id OR UL.client_id = ISNULL((SELECT defaultCatalog_id from CatalogType), 0)) 
            ) ULOverride
        WHERE RowNbr=1 
);

Results in:

    public partial class ItvfClientCostCatalogTestResult
    {
        [Column("code")]
        public string Code { get; set; } = default!;
        [Column("uniformat_level_id")]
        public int UniformatLevelId { get; set; }
        [Column("client_id")]
        public int ClientId { get; set; }
        [Column("name")]
        public string Name { get; set; } = default!;
        public string? Comment { get; set; }
        public long? RowNbr { get; set; }
    }

Whereas:

public partial class UniformatLevel
{
    [Key]
    [Column("uniformat_level_id")]
    public int UniformatLevelId { get; set; }

    [Column("code")]
    [StringLength(50)]
    public string Code { get; set; } = null!;

    [Column("name")]
    [StringLength(250)]
    public string Name { get; set; } = null!;

    [Column("client_id")]
    public int ClientId { get; set; }

    [StringLength(100)]
    public string? Comment { get; set; }
}
ErikEJ commented 1 day ago

@BillDuffy How does your "code uses that to limit string lengths" look like?

BillDuffy commented 1 day ago
            if (type == typeof(string))
            {
                if (secureProperty.GetCustomAttribute(typeof(StringLengthAttribute)) is StringLengthAttribute prop)
                {
                    var value = (sourceVal as string) ?? string.Empty;

                    if (prop.MaximumLength > 0 && value.Length > prop.MaximumLength)
                        throw new ConstraintException($"Property {secureProperty.Name} length:{value.Length} > allowed maximum {prop.MaximumLength}");

                    if (prop.MinimumLength > 0 && value.Length < prop.MinimumLength)
                        throw new ConstraintException($"Property {secureProperty.Name} length:{value.Length} < allowed minimum {prop.MinimumLength}");
                }
            }

            else if (secureProperty.PropertyType is { IsArray: true, HasElementType: true })
            {
                if (secureProperty.GetCustomAttribute(typeof(MaxLengthAttribute)) is MaxLengthAttribute prop &&
                    prop.Length > 0 &&
                    sourceVal is Array array)
                {
                    if (array.Length > prop.Length)
                        throw new ConstraintException($"Property {secureProperty.Name} length:{array.Length} > allowed maximum {prop.Length}");
                }
            }
ErikEJ commented 1 day ago

@BillDuffy Would adding this if UseDecimalDataAnnotationForSprocResult = true work for you? (I do not want it to be on for everyone)

BillDuffy commented 1 day ago

I currently have that turned on and, yes using that key to enable this would be great.

ErikEJ commented 20 hours ago

@BillDuffy I implemented a fix for this in the latest daily build, would be grateful if you could try it out.

BillDuffy commented 16 hours ago

Errors occurred whilst processing Stored Procedures: image

Failed to produce ...Functions.cs file.

Following are the tables needed to create the dbo.sp_ImpactTypeChoices Stored Procedure:

CREATE TABLE [dbo].[impact_type] (
    [impact_type_id] INT           NOT NULL,
    [name]           NVARCHAR (50) NOT NULL,
);
CREATE TABLE [dbo].[impact_severity] (
    [impact_severity_id] INT            IDENTITY (10000, 1) NOT NULL,
    [impact_type_id]     INT            NOT NULL,
    [name]               NVARCHAR (255) NOT NULL,
    [sort_order]         TINYINT        NULL,
    [description]        NVARCHAR (255) NULL,
);
CREATE TABLE [dbo].[deficiency_score_matrix] (
    [deficiency_score_matrix_id]       INT            IDENTITY (100, 1) NOT NULL,
    [deficiency_mishap_probability_id] INT            NOT NULL,
    [impact_severity_id]               INT            NOT NULL,
    [score]                            DECIMAL (9, 5) NOT NULL,
    [create_date]                      DATETIME2 (2) ,
    [create_security_user_id]          INT            NOT NULL,
    [update_date]                      DATETIME2 (2)  NULL,
    [update_security_user_id]          INT            NULL,
);
CREATE TABLE [dbo].[deficiency_mishap_probability] (
    [deficiency_mishap_probability_id] INT            NOT NULL,
    [category]                         CHAR (1)       NOT NULL,
    [name]                             NVARCHAR (100) NOT NULL,
);
CREATE PROCEDURE [dbo].[sp_ImpactTypeChoices]
AS
BEGIN
    SET NOCOUNT ON;

    SELECT IT.name 'ImpactType'
            , ISEV.NAME 'Severity'
            , ISEV.description -- 'Severity Description'
            , ('Subcategory - ' + DMP.category) 'category' 
            , DMP.name 'FailureProbabilityDescription'
            , DSM.score 
            , DSM.deficiency_score_matrix_id 
    FROM impact_type IT 
        INNER JOIN impact_severity ISEV 
            ON IT.impact_type_id = ISEV.impact_type_id
        INNER JOIN deficiency_score_matrix DSM 
            ON ISEV.impact_severity_id = DSM.impact_severity_id
        INNER JOIN deficiency_mishap_probability DMP 
            ON DMP.deficiency_mishap_probability_id = DSM.deficiency_mishap_probability_id
    ORDER BY IT.name, ISEV.name, DMP.category  
END
ErikEJ commented 15 hours ago

Which build exactly??

Most likely a bug in latest daily...

BillDuffy commented 15 hours ago

Images of other problems:

image

image

image

image

BillDuffy commented 15 hours ago

image

ErikEJ commented 5 hours ago

Re-open to do some smoke testing

ErikEJ commented 3 hours ago

I implemented a fix for this in the latest daily build, would be grateful if you could try it out.