Skip to content

Commit

Permalink
[ODS-6018] Enforcement of uniqueness in collections (#843)
Browse files Browse the repository at this point in the history 
* Fixed object equality implementation for resource classes, added support for correct case-sensitivity comparison for strings and descriptor values, and removed NoDuplicateMembers validation attribute from the entity classes, as well as the Distinct invocation during collection item mapping (as it is no longer necessary).

* Updated approval tests.

* Added Postman test coverage for validation for duplicate collection items.

* Re-added newline at end of NoDuplicateMembersAttribute.
  • Loading branch information
@gmcelhanon
gmcelhanon authored Oct 9, 2023
1 parent e788490 commit b4540f7
Show file tree
Hide file tree
Showing 27 changed files with 2,359 additions and 2,234 deletions.
48 changes: 22 additions & 26 deletions Application/EdFi.Ods.Common/Attributes/NoDuplicateMembersAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,39 @@
using System.Collections;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using EdFi.Common.Inflection;

namespace EdFi.Ods.Common.Attributes
namespace EdFi.Ods.Common.Attributes;

public sealed class NoDuplicateMembersAttribute : ValidationAttribute
{
public sealed class NoDuplicateMembersAttribute : ValidationAttribute
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
var enumerable = value as IEnumerable;
var enumerable = value as IEnumerable;

if (enumerable == null)
{
return ValidationResult.Success;
}
if (enumerable == null)
{
return ValidationResult.Success;
}

var i = 0;
var enumerableHashSet = new HashSet<object>();
var itemNumber = 1;
var enumerableHashSet = new HashSet<object>();

foreach (var item in enumerable)
foreach (var item in enumerable)
{
if (item != null)
{
if (item != null)
if (!enumerableHashSet.Add(item))
{
if (!enumerableHashSet.Add(item))
{
return
new ValidationResult(
string.Format(
"{0} enumerable contains duplicate at index: {1}",
validationContext.DisplayName,
i));
}
return new ValidationResult(
$"The {Inflector.AddOrdinalSuffix(itemNumber.ToString())} item of the {validationContext.DisplayName} collection is a duplicate.");
}

i++;
}

// If we're still here, validation was successful
return ValidationResult.Success;
itemNumber++;
}

// If we're still here, validation was successful
return ValidationResult.Success;
}
}
2 changes: 1 addition & 1 deletion Application/EdFi.Ods.Common/Extensions/CollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public static void MapCollectionTo<TSource, TTarget>(
var targetListType = targetList.GetType();
var itemType = GetItemType();

foreach (var sourceItem in sourceList.Distinct().Where(i => isItemIncluded == null || isItemIncluded(i)))
foreach (var sourceItem in sourceList.Where(i => isItemIncluded == null || isItemIncluded(i)))
{
var targetItem = (TTarget) Activator.CreateInstance(itemType);

Expand Down
67 changes: 67 additions & 0 deletions Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8748,6 +8748,73 @@
}
]
},
{
"name": "Duplicate collection items",
"item": [
{
"name": "Create assessment with duplicate items",
"event": [
{
"listen": "test",
"script": {
"exec": [
"pm.test(\"Response code is 400\", () => {\r",
" pm.expect(pm.response.code).to.equal(400);\r",
"});\r",
"\r",
"const error = pm.response.json();\r",
"\r",
"pm.test(\"Message should indicate the request is invalid.\", () => {\r",
" pm.expect(error.message).to.equal(\"The request is invalid.\");\r",
"});\r",
"\r",
"pm.test(\"Model state should include validation messages for duplicate items in collections.\", () => {\r",
" pm.expect(error.modelState).to.deep.include({\r",
" \"request.AssessmentScores\": [\r",
" \"The 2nd item of the AssessmentScores collection is a duplicate.\"\r",
" ],\r",
" \"request.AssessmentAcademicSubjects\": [\r",
" \"The 3rd item of the AssessmentAcademicSubjects collection is a duplicate.\"\r",
" ],\r",
" \"request.AssessmentAssessedGradeLevels\": [\r",
" \"The 3rd item of the AssessmentAssessedGradeLevels collection is a duplicate.\"\r",
" ]\r",
" });\r",
"});"
],
"type": "text/javascript"
}
}
],
"request": {
"method": "POST",
"header": [],
"body": {
"mode": "raw",
"raw": "{\r\n \"assessmentIdentifier\": \"{{$guid}}\",\r\n \"namespace\": \"uri://ed-fi.org/Assessment/Assessment.xml\",\r\n \"assessmentTitle\": \"{{$randomCompanyName}}\",\r\n \"academicSubjects\": [\r\n {\r\n \"academicSubjectDescriptor\": \"uri://ed-fi.org/AcademicSubjectDescriptor#Mathematics\"\r\n },\r\n {\r\n \"academicSubjectDescriptor\": \"uri://ed-fi.org/AcademicSubjectDescriptor#English Language Arts\"\r\n },\r\n {\r\n \"academicSubjectDescriptor\": \"uri://ed-fi.org/AcademicSubjectDescriptor#Mathematics\"\r\n }\r\n ],\r\n \"assessedGradeLevels\": [\r\n {\r\n \"gradeLevelDescriptor\": \"uri://ed-fi.org/GradeLevelDescriptor#Fourth grade\"\r\n },\r\n {\r\n \"gradeLevelDescriptor\": \"uri://ed-fi.org/GradeLevelDescriptor#Third grade\"\r\n },\r\n {\r\n \"gradeLevelDescriptor\": \"uri://ed-fi.org/GradeLevelDescriptor#Fourth grade\"\r\n }\r\n ],\r\n \"identificationCodes\": [\r\n {\r\n \"assessmentIdentificationSystemDescriptor\": \"uri://ed-fi.org/AssessmentIdentificationSystemDescriptor#Test Contractor\",\r\n \"identificationCode\": \"01774fa3-06f1-47fe-8801-c8b1e65057f2\"\r\n }\r\n ],\r\n \"languages\": [\r\n {\r\n \"languageDescriptor\": \"uri://ed-fi.org/LanguageDescriptor#eng\"\r\n }\r\n ],\r\n \"performanceLevels\": [],\r\n \"periods\": [],\r\n \"platformTypes\": [],\r\n \"programs\": [],\r\n \"scores\": [\r\n {\r\n \"assessmentReportingMethodDescriptor\": \"uri://ed-fi.org/AssessmentReportingMethodDescriptor#Raw score\",\r\n \"maximumScore\": \"10\",\r\n \"minimumScore\": \"0\",\r\n \"resultDatatypeTypeDescriptor\": \"uri://ed-fi.org/ResultDatatypeTypeDescriptor#Integer\"\r\n },\r\n {\r\n \"assessmentReportingMethodDescriptor\": \"uri://ed-fi.org/AssessmentReportingMethodDescriptor#Raw score\",\r\n \"maximumScore\": \"5\",\r\n \"minimumScore\": \"0\",\r\n \"resultDatatypeTypeDescriptor\": \"uri://ed-fi.org/ResultDatatypeTypeDescriptor#Integer\"\r\n }\r\n ],\r\n \"sections\": [],\r\n \"contentStandard\": {\r\n \"title\": \"State Essential Knowledge and Skills\",\r\n \"authors\": []\r\n }\r\n}",
"options": {
"raw": {
"language": "json"
}
}
},
"url": {
"raw": "{{ApiBaseUrl}}/data/v3/ed-fi/assessments",
"host": [
"{{ApiBaseUrl}}"
],
"path": [
"data",
"v3",
"ed-fi",
"assessments"
]
}
},
"response": []
}
]
},
{
"name": "StudentProgramAssociations",
"item": [
Expand Down
8 changes: 4 additions & 4 deletions ...erify.Extensions.Homograph.1.0.0_Std_4.0.0_Models_Entities_Entities.generated.approved.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ string Entities.Common.Homograph.IParent.ParentNameDiscriminator
private ICollection<Entities.NHibernate.ParentAggregate.Homograph.ParentAddress> _parentAddresses;
private ICollection<Entities.Common.Homograph.IParentAddress> _parentAddressesCovariant;
[RequiredCollection]
[ValidateEnumerable, NoDuplicateMembers]
[ValidateEnumerable]
public virtual ICollection<Entities.NHibernate.ParentAggregate.Homograph.ParentAddress> ParentAddresses
{
get
Expand Down Expand Up @@ -465,7 +465,7 @@ public virtual ICollection<Entities.NHibernate.ParentAggregate.Homograph.ParentA
private ICollection<Entities.NHibernate.ParentAggregate.Homograph.ParentStudentSchoolAssociation> _parentStudentSchoolAssociations;
private ICollection<Entities.Common.Homograph.IParentStudentSchoolAssociation> _parentStudentSchoolAssociationsCovariant;
[RequiredCollection]
[ValidateEnumerable, NoDuplicateMembers]
[ValidateEnumerable]
public virtual ICollection<Entities.NHibernate.ParentAggregate.Homograph.ParentStudentSchoolAssociation> ParentStudentSchoolAssociations
{
get
Expand Down Expand Up @@ -1805,7 +1805,7 @@ string Entities.Common.Homograph.IStaff.StaffNameDiscriminator

private ICollection<Entities.NHibernate.StaffAggregate.Homograph.StaffAddress> _staffAddresses;
private ICollection<Entities.Common.Homograph.IStaffAddress> _staffAddressesCovariant;
[ValidateEnumerable, NoDuplicateMembers]
[ValidateEnumerable]
public virtual ICollection<Entities.NHibernate.StaffAggregate.Homograph.StaffAddress> StaffAddresses
{
get
Expand Down Expand Up @@ -1853,7 +1853,7 @@ public virtual ICollection<Entities.NHibernate.StaffAggregate.Homograph.StaffAdd

private ICollection<Entities.NHibernate.StaffAggregate.Homograph.StaffStudentSchoolAssociation> _staffStudentSchoolAssociations;
private ICollection<Entities.Common.Homograph.IStaffStudentSchoolAssociation> _staffStudentSchoolAssociationsCovariant;
[ValidateEnumerable, NoDuplicateMembers]
[ValidateEnumerable]
public virtual ICollection<Entities.NHibernate.StaffAggregate.Homograph.StaffStudentSchoolAssociation> StaffStudentSchoolAssociations
{
get
Expand Down
44 changes: 22 additions & 22 deletions ...sts.Verify.Extensions.Homograph.1.0.0_Std_4.0.0_Resources_Resources.generated.approved.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,12 @@ public override bool Equals(object obj)


// Standard Property
if ((this as Entities.Common.Homograph.IName).FirstName.Equals(compareTo.FirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IName).FirstName, compareTo.FirstName))
return false;


// Standard Property
if ((this as Entities.Common.Homograph.IName).LastSurname.Equals(compareTo.LastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IName).LastSurname, compareTo.LastSurname))
return false;


Expand Down Expand Up @@ -574,12 +574,12 @@ public override bool Equals(object obj)


// Referenced Property
if (!(this as Entities.Common.Homograph.IParent).ParentFirstName.Equals(compareTo.ParentFirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IParent).ParentFirstName, compareTo.ParentFirstName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IParent).ParentLastSurname.Equals(compareTo.ParentLastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IParent).ParentLastSurname, compareTo.ParentLastSurname))
return false;


Expand Down Expand Up @@ -950,7 +950,7 @@ public override bool Equals(object obj)


// Standard Property
if ((this as Entities.Common.Homograph.IParentAddress).City.Equals(compareTo.City))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IParentAddress).City, compareTo.City))
return false;


Expand Down Expand Up @@ -1273,17 +1273,17 @@ public override bool Equals(object obj)


// Referenced Property
if (!(this as Entities.Common.Homograph.IParentStudentSchoolAssociation).SchoolName.Equals(compareTo.SchoolName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IParentStudentSchoolAssociation).SchoolName, compareTo.SchoolName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IParentStudentSchoolAssociation).StudentFirstName.Equals(compareTo.StudentFirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IParentStudentSchoolAssociation).StudentFirstName, compareTo.StudentFirstName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IParentStudentSchoolAssociation).StudentLastSurname.Equals(compareTo.StudentLastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IParentStudentSchoolAssociation).StudentLastSurname, compareTo.StudentLastSurname))
return false;


Expand Down Expand Up @@ -1627,7 +1627,7 @@ public override bool Equals(object obj)


// Standard Property
if ((this as Entities.Common.Homograph.ISchool).SchoolName.Equals(compareTo.SchoolName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.ISchool).SchoolName, compareTo.SchoolName))
return false;


Expand Down Expand Up @@ -2189,7 +2189,7 @@ public override bool Equals(object obj)


// Standard Property
if ((this as Entities.Common.Homograph.ISchoolYearType).SchoolYear.Equals(compareTo.SchoolYear))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.ISchoolYearType).SchoolYear, compareTo.SchoolYear))
return false;


Expand Down Expand Up @@ -2569,12 +2569,12 @@ public override bool Equals(object obj)


// Referenced Property
if (!(this as Entities.Common.Homograph.IStaff).StaffFirstName.Equals(compareTo.StaffFirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStaff).StaffFirstName, compareTo.StaffFirstName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IStaff).StaffLastSurname.Equals(compareTo.StaffLastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStaff).StaffLastSurname, compareTo.StaffLastSurname))
return false;


Expand Down Expand Up @@ -2945,7 +2945,7 @@ public override bool Equals(object obj)


// Standard Property
if ((this as Entities.Common.Homograph.IStaffAddress).City.Equals(compareTo.City))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStaffAddress).City, compareTo.City))
return false;


Expand Down Expand Up @@ -3268,17 +3268,17 @@ public override bool Equals(object obj)


// Referenced Property
if (!(this as Entities.Common.Homograph.IStaffStudentSchoolAssociation).SchoolName.Equals(compareTo.SchoolName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStaffStudentSchoolAssociation).SchoolName, compareTo.SchoolName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IStaffStudentSchoolAssociation).StudentFirstName.Equals(compareTo.StudentFirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStaffStudentSchoolAssociation).StudentFirstName, compareTo.StudentFirstName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IStaffStudentSchoolAssociation).StudentLastSurname.Equals(compareTo.StudentLastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStaffStudentSchoolAssociation).StudentLastSurname, compareTo.StudentLastSurname))
return false;


Expand Down Expand Up @@ -3700,12 +3700,12 @@ public override bool Equals(object obj)


// Referenced Property
if (!(this as Entities.Common.Homograph.IStudent).StudentFirstName.Equals(compareTo.StudentFirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStudent).StudentFirstName, compareTo.StudentFirstName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IStudent).StudentLastSurname.Equals(compareTo.StudentLastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStudent).StudentLastSurname, compareTo.StudentLastSurname))
return false;


Expand Down Expand Up @@ -4000,7 +4000,7 @@ public override bool Equals(object obj)


// Standard Property
if ((this as Entities.Common.Homograph.IStudentAddress).City.Equals(compareTo.City))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStudentAddress).City, compareTo.City))
return false;


Expand Down Expand Up @@ -4431,17 +4431,17 @@ public override bool Equals(object obj)


// Referenced Property
if (!(this as Entities.Common.Homograph.IStudentSchoolAssociation).SchoolName.Equals(compareTo.SchoolName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStudentSchoolAssociation).SchoolName, compareTo.SchoolName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IStudentSchoolAssociation).StudentFirstName.Equals(compareTo.StudentFirstName))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStudentSchoolAssociation).StudentFirstName, compareTo.StudentFirstName))
return false;


// Referenced Property
if (!(this as Entities.Common.Homograph.IStudentSchoolAssociation).StudentLastSurname.Equals(compareTo.StudentLastSurname))
if (!GeneratedArtifactStaticDependencies.DatabaseEngineSpecificStringComparer.Equals((this as Entities.Common.Homograph.IStudentSchoolAssociation).StudentLastSurname, compareTo.StudentLastSurname))
return false;


Expand Down
Loading

0 comments on commit b4540f7

Please sign in to comment.