From 44c6aabb1c6ef590a96f096c42c42e296b744d03 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Fri, 14 Apr 2017 21:34:38 +0200 Subject: [PATCH] Field reordering in schema. --- src/Squidex.Core/ContentExtensions.cs | 4 +- src/Squidex.Core/Contents/ContentData.cs | 2 +- .../Schemas/Json/JsonFieldModel.cs | 2 + .../Schemas/Json/JsonSchemaModel.cs | 2 +- .../Schemas/Json/SchemaJsonSerializer.cs | 31 ++++------ src/Squidex.Core/Schemas/Schema.cs | 61 ++++++++++++++----- .../Schemas/SchemaCommandHandler.cs | 2 +- .../Schemas/SchemaDomainObject.cs | 2 +- .../Models/Converters/SchemaConverter.cs | 8 +-- .../Squidex.Core.Tests/Schemas/SchemaTests.cs | 56 ++++++++++++++--- .../Schemas/SchemaDomainObjectTests.cs | 14 ++--- 11 files changed, 127 insertions(+), 57 deletions(-) diff --git a/src/Squidex.Core/ContentExtensions.cs b/src/Squidex.Core/ContentExtensions.cs index 3decba2bf..7ad7ac1b7 100644 --- a/src/Squidex.Core/ContentExtensions.cs +++ b/src/Squidex.Core/ContentExtensions.cs @@ -18,9 +18,9 @@ namespace Squidex.Core { public static ContentData Enrich(this ContentData data, Schema schema, HashSet languages) { - var validator = new ContentEnricher(languages, schema); + var enricher = new ContentEnricher(languages, schema); - validator.Enrich(data); + enricher.Enrich(data); return data; } diff --git a/src/Squidex.Core/Contents/ContentData.cs b/src/Squidex.Core/Contents/ContentData.cs index 283dd52a4..a3715ebdd 100644 --- a/src/Squidex.Core/Contents/ContentData.cs +++ b/src/Squidex.Core/Contents/ContentData.cs @@ -111,7 +111,7 @@ namespace Squidex.Core.Contents foreach (var fieldValue in this) { - if (!long.TryParse(fieldValue.Key, out long fieldId) || !schema.Fields.TryGetValue(fieldId, out Field field)) + if (!long.TryParse(fieldValue.Key, out long fieldId) || !schema.FieldsById.TryGetValue(fieldId, out Field field)) { continue; } diff --git a/src/Squidex.Core/Schemas/Json/JsonFieldModel.cs b/src/Squidex.Core/Schemas/Json/JsonFieldModel.cs index 8697a94ea..6c7767802 100644 --- a/src/Squidex.Core/Schemas/Json/JsonFieldModel.cs +++ b/src/Squidex.Core/Schemas/Json/JsonFieldModel.cs @@ -12,6 +12,8 @@ namespace Squidex.Core.Schemas.Json { public string Name { get; set; } + public long Id { get; set; } + public bool IsHidden { get; set; } public bool IsDisabled { get; set; } diff --git a/src/Squidex.Core/Schemas/Json/JsonSchemaModel.cs b/src/Squidex.Core/Schemas/Json/JsonSchemaModel.cs index 3df33cfad..55c964c25 100644 --- a/src/Squidex.Core/Schemas/Json/JsonSchemaModel.cs +++ b/src/Squidex.Core/Schemas/Json/JsonSchemaModel.cs @@ -18,6 +18,6 @@ namespace Squidex.Core.Schemas.Json public SchemaProperties Properties { get; set; } - public Dictionary Fields { get; set; } + public List Fields { get; set; } } } \ No newline at end of file diff --git a/src/Squidex.Core/Schemas/Json/SchemaJsonSerializer.cs b/src/Squidex.Core/Schemas/Json/SchemaJsonSerializer.cs index 31dd33e94..6af61337a 100644 --- a/src/Squidex.Core/Schemas/Json/SchemaJsonSerializer.cs +++ b/src/Squidex.Core/Schemas/Json/SchemaJsonSerializer.cs @@ -6,7 +6,6 @@ // All rights reserved. // ========================================================================== -using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Newtonsoft.Json; @@ -36,18 +35,16 @@ namespace Squidex.Core.Schemas.Json { var model = new JsonSchemaModel { Name = schema.Name, IsPublished = schema.IsPublished, Properties = schema.Properties }; - model.Fields = - schema.Fields - .Select(x => - new KeyValuePair(x.Key, - new JsonFieldModel - { - Name = x.Value.Name, - IsHidden = x.Value.IsHidden, - IsDisabled = x.Value.IsDisabled, - Properties = x.Value.RawProperties - })) - .ToDictionary(x => x.Key, x => x.Value); + model.Fields = + schema.Fields.Select(x => + new JsonFieldModel + { + Id = x.Id, + Name = x.Name, + IsHidden = x.IsHidden, + IsDisabled = x.IsDisabled, + Properties = x.RawProperties + }).ToList(); return JToken.FromObject(model, serializer); } @@ -57,11 +54,9 @@ namespace Squidex.Core.Schemas.Json var model = token.ToObject(serializer); var fields = - model.Fields.Select(kvp => + model.Fields.Select(fieldModel => { - var fieldModel = kvp.Value; - - var field = fieldRegistry.CreateField(kvp.Key, fieldModel.Name, fieldModel.Properties); + var field = fieldRegistry.CreateField(fieldModel.Id, fieldModel.Name, fieldModel.Properties); if (fieldModel.IsDisabled) { @@ -74,7 +69,7 @@ namespace Squidex.Core.Schemas.Json } return field; - }).ToImmutableDictionary(x => x.Id, x => x); + }).ToImmutableList(); var schema = new Schema( diff --git a/src/Squidex.Core/Schemas/Schema.cs b/src/Squidex.Core/Schemas/Schema.cs index 9a3ac2b26..08c438d4b 100644 --- a/src/Squidex.Core/Schemas/Schema.cs +++ b/src/Squidex.Core/Schemas/Schema.cs @@ -23,6 +23,7 @@ namespace Squidex.Core.Schemas { private readonly string name; private readonly SchemaProperties properties; + private readonly ImmutableList fields; private readonly ImmutableDictionary fieldsById; private readonly ImmutableDictionary fieldsByName; private readonly bool isPublished; @@ -37,7 +38,12 @@ namespace Squidex.Core.Schemas get { return isPublished; } } - public ImmutableDictionary Fields + public ImmutableList Fields + { + get { return fields; } + } + + public ImmutableDictionary FieldsById { get { return fieldsById; } } @@ -52,17 +58,19 @@ namespace Squidex.Core.Schemas get { return properties; } } - public Schema(string name, bool isPublished, SchemaProperties properties, ImmutableDictionary fields) + public Schema(string name, bool isPublished, SchemaProperties properties, ImmutableList fields) { Guard.NotNull(fields, nameof(fields)); Guard.NotNull(properties, nameof(properties)); Guard.ValidSlug(name, nameof(name)); - fieldsById = fields; - fieldsByName = fields.Values.ToImmutableDictionary(x => x.Name, StringComparer.OrdinalIgnoreCase); + fieldsById = fields.ToImmutableDictionary(x => x.Id); + fieldsByName = fields.ToImmutableDictionary(x => x.Name, StringComparer.OrdinalIgnoreCase); this.name = name; + this.fields = fields; + this.properties = properties; this.properties.Freeze(); @@ -78,14 +86,14 @@ namespace Squidex.Core.Schemas throw new ValidationException("Cannot create a new schema", error); } - return new Schema(name, false, newProperties, ImmutableDictionary.Empty); + return new Schema(name, false, newProperties, ImmutableList.Empty); } public Schema Update(SchemaProperties newProperties) { Guard.NotNull(newProperties, nameof(newProperties)); - return new Schema(name, isPublished, newProperties, fieldsById); + return new Schema(name, isPublished, newProperties, fields); } public Schema UpdateField(long fieldId, FieldProperties newProperties) @@ -120,7 +128,7 @@ namespace Squidex.Core.Schemas public Schema DeleteField(long fieldId) { - return new Schema(name, isPublished, properties, fieldsById.Remove(fieldId)); + return new Schema(name, isPublished, properties, fields.Where(x => x.Id != fieldId).ToImmutableList()); } public Schema Publish() @@ -130,7 +138,7 @@ namespace Squidex.Core.Schemas throw new DomainException("Schema is already published"); } - return new Schema(name, true, properties, fieldsById); + return new Schema(name, true, properties, fields); } public Schema Unpublish() @@ -140,19 +148,21 @@ namespace Squidex.Core.Schemas throw new DomainException("Schema is not published"); } - return new Schema(name, false, properties, fieldsById); + return new Schema(name, false, properties, fields); } - public Schema AddOrUpdateField(Field field) + public Schema ReorderFields(List ids) { - Guard.NotNull(field, nameof(field)); + Guard.NotNull(ids, nameof(ids)); - if (fieldsById.Values.Any(f => f.Name == field.Name && f.Id != field.Id)) + if (ids.Count != fields.Count || ids.Any(x => !fieldsById.ContainsKey(x))) { - throw new ValidationException($"A field with name '{field.Name}' already exists."); + throw new ArgumentException("Ids must cover all fields.", nameof(ids)); } - return new Schema(name, isPublished, properties, fieldsById.SetItem(field.Id, field)); + var newFields = fields.OrderBy(f => ids.IndexOf(f.Id)).ToImmutableList(); + + return new Schema(name, isPublished, properties, newFields); } public Schema UpdateField(long fieldId, Func updater) @@ -169,6 +179,29 @@ namespace Squidex.Core.Schemas return AddOrUpdateField(newField); } + public Schema AddOrUpdateField(Field field) + { + Guard.NotNull(field, nameof(field)); + + if (fieldsById.Values.Any(f => f.Name == field.Name && f.Id != field.Id)) + { + throw new ValidationException($"A field with name '{field.Name}' already exists."); + } + + ImmutableList newFields; + + if (fieldsById.ContainsKey(field.Id)) + { + newFields = fields.Select(f => f.Id == field.Id ? field : f).ToImmutableList(); + } + else + { + newFields = fields.Add(field); + } + + return new Schema(name, isPublished, properties, newFields); + } + public EdmComplexType BuildEdmType(HashSet languages, Func typeResolver) { Guard.NotEmpty(languages, nameof(languages)); diff --git a/src/Squidex.Write/Schemas/SchemaCommandHandler.cs b/src/Squidex.Write/Schemas/SchemaCommandHandler.cs index d4af27a0f..8b5f0de87 100644 --- a/src/Squidex.Write/Schemas/SchemaCommandHandler.cs +++ b/src/Squidex.Write/Schemas/SchemaCommandHandler.cs @@ -56,7 +56,7 @@ namespace Squidex.Write.Schemas { s.AddField(command); - context.Succeed(EntityCreatedResult.Create(s.Schema.Fields.Values.First(x => x.Name == command.Name).Id, s.Version)); + context.Succeed(EntityCreatedResult.Create(s.Schema.FieldsById.Values.First(x => x.Name == command.Name).Id, s.Version)); }); } diff --git a/src/Squidex.Write/Schemas/SchemaDomainObject.cs b/src/Squidex.Write/Schemas/SchemaDomainObject.cs index bda7fdab9..51279aec5 100644 --- a/src/Squidex.Write/Schemas/SchemaDomainObject.cs +++ b/src/Squidex.Write/Schemas/SchemaDomainObject.cs @@ -240,7 +240,7 @@ namespace Squidex.Write.Schemas { SimpleMapper.Map(fieldCommand, @event); - if (schema.Fields.TryGetValue(fieldCommand.FieldId, out Field field)) + if (schema.FieldsById.TryGetValue(fieldCommand.FieldId, out Field field)) { @event.FieldId = new NamedId(field.Id, field.Name); } diff --git a/src/Squidex/Controllers/Api/Schemas/Models/Converters/SchemaConverter.cs b/src/Squidex/Controllers/Api/Schemas/Models/Converters/SchemaConverter.cs index 2c1e81d17..d399f1447 100644 --- a/src/Squidex/Controllers/Api/Schemas/Models/Converters/SchemaConverter.cs +++ b/src/Squidex/Controllers/Api/Schemas/Models/Converters/SchemaConverter.cs @@ -55,12 +55,12 @@ namespace Squidex.Controllers.Api.Schemas.Models.Converters dto.Fields = new List(); - foreach (var kvp in entity.Schema.Fields) + foreach (var field in entity.Schema.Fields) { - var fieldPropertiesDto = Factories[kvp.Value.RawProperties.GetType()](kvp.Value.RawProperties); - var fieldDto = SimpleMapper.Map(kvp.Value, new FieldDto { FieldId = kvp.Key, Properties = fieldPropertiesDto }); + var fieldPropertiesDto = Factories[field.RawProperties.GetType()](field.RawProperties); + var fieldInstanceDto = SimpleMapper.Map(field, new FieldDto { FieldId = field.Id, Properties = fieldPropertiesDto }); - dto.Fields.Add(fieldDto); + dto.Fields.Add(fieldInstanceDto); } return dto; diff --git a/tests/Squidex.Core.Tests/Schemas/SchemaTests.cs b/tests/Squidex.Core.Tests/Schemas/SchemaTests.cs index c2dfc7edb..35b8c8242 100644 --- a/tests/Squidex.Core.Tests/Schemas/SchemaTests.cs +++ b/tests/Squidex.Core.Tests/Schemas/SchemaTests.cs @@ -9,6 +9,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using Newtonsoft.Json.Linq; using NJsonSchema; using Squidex.Infrastructure; @@ -65,7 +66,7 @@ namespace Squidex.Core.Schemas { var field = AddField(); - Assert.Equal(field, sut.Fields[1]); + Assert.Equal(field, sut.FieldsById[1]); } [Fact] @@ -84,7 +85,7 @@ namespace Squidex.Core.Schemas sut = sut.HideField(1); sut = sut.HideField(1); - Assert.True(sut.Fields[1].IsHidden); + Assert.True(sut.FieldsById[1].IsHidden); } [Fact] @@ -102,7 +103,7 @@ namespace Squidex.Core.Schemas sut = sut.ShowField(1); sut = sut.ShowField(1); - Assert.False(sut.Fields[1].IsHidden); + Assert.False(sut.FieldsById[1].IsHidden); } [Fact] @@ -119,7 +120,7 @@ namespace Squidex.Core.Schemas sut = sut.DisableField(1); sut = sut.DisableField(1); - Assert.True(sut.Fields[1].IsDisabled); + Assert.True(sut.FieldsById[1].IsDisabled); } [Fact] @@ -137,7 +138,7 @@ namespace Squidex.Core.Schemas sut = sut.EnableField(1); sut = sut.EnableField(1); - Assert.False(sut.Fields[1].IsDisabled); + Assert.False(sut.FieldsById[1].IsDisabled); } [Fact] @@ -153,7 +154,7 @@ namespace Squidex.Core.Schemas sut = sut.RenameField(1, "new-name"); - Assert.Equal("new-name", sut.Fields[1].Name); + Assert.Equal("new-name", sut.FieldsById[1].Name); } [Fact] @@ -187,7 +188,7 @@ namespace Squidex.Core.Schemas sut = sut.DeleteField(1); - Assert.Equal(0, sut.Fields.Count); + Assert.Equal(0, sut.FieldsById.Count); } [Fact] @@ -203,7 +204,7 @@ namespace Squidex.Core.Schemas sut = sut.UpdateField(1, new NumberFieldProperties { Hints = "my-hints" }); - Assert.Equal("my-hints", sut.Fields[1].RawProperties.Hints); + Assert.Equal("my-hints", sut.FieldsById[1].RawProperties.Hints); } [Fact] @@ -251,6 +252,45 @@ namespace Squidex.Core.Schemas Assert.Throws(() => sut.Unpublish()); } + [Fact] + public void Should_reorder_fields() + { + var field1 = new StringField(1, "1", new StringFieldProperties()); + var field2 = new StringField(2, "2", new StringFieldProperties()); + var field3 = new StringField(3, "3", new StringFieldProperties()); + + sut = sut.AddOrUpdateField(field1); + sut = sut.AddOrUpdateField(field2); + sut = sut.AddOrUpdateField(field3); + sut = sut.ReorderFields(new List { 3, 2, 1 }); + + Assert.Equal(new List { field3, field2, field1 }, sut.Fields.ToList()); + } + + [Fact] + public void Should_throw_if_not_all_fields_are_covered_for_reordering() + { + var field1 = new StringField(1, "1", new StringFieldProperties()); + var field2 = new StringField(2, "2", new StringFieldProperties()); + + sut = sut.AddOrUpdateField(field1); + sut = sut.AddOrUpdateField(field2); + + Assert.Throws(() => sut.ReorderFields(new List { 1 })); + } + + [Fact] + public void Should_throw_if_field_to_reorder_does_not_exist() + { + var field1 = new StringField(1, "1", new StringFieldProperties()); + var field2 = new StringField(2, "2", new StringFieldProperties()); + + sut = sut.AddOrUpdateField(field1); + sut = sut.AddOrUpdateField(field2); + + Assert.Throws(() => sut.ReorderFields(new List { 1, 4 })); + } + [Fact] public void Should_build_schema() { diff --git a/tests/Squidex.Write.Tests/Schemas/SchemaDomainObjectTests.cs b/tests/Squidex.Write.Tests/Schemas/SchemaDomainObjectTests.cs index 022ade096..14fbf43e0 100644 --- a/tests/Squidex.Write.Tests/Schemas/SchemaDomainObjectTests.cs +++ b/tests/Squidex.Write.Tests/Schemas/SchemaDomainObjectTests.cs @@ -265,7 +265,7 @@ namespace Squidex.Write.Schemas sut.AddField(CreateCommand(new AddField { Name = fieldName, Properties = properties })); - Assert.Equal(properties, sut.Schema.Fields[1].RawProperties); + Assert.Equal(properties, sut.Schema.FieldsById[1].RawProperties); sut.GetUncomittedEvents() .ShouldHaveSameEvents( @@ -324,7 +324,7 @@ namespace Squidex.Write.Schemas sut.UpdateField(CreateCommand(new UpdateField { FieldId = 1, Properties = properties })); - Assert.Equal(properties, sut.Schema.Fields[1].RawProperties); + Assert.Equal(properties, sut.Schema.FieldsById[1].RawProperties); sut.GetUncomittedEvents() .ShouldHaveSameEvents( @@ -372,7 +372,7 @@ namespace Squidex.Write.Schemas sut.HideField(CreateCommand(new HideField { FieldId = 1 })); - Assert.True(sut.Schema.Fields[1].IsHidden); + Assert.True(sut.Schema.FieldsById[1].IsHidden); sut.GetUncomittedEvents() .ShouldHaveSameEvents( @@ -421,7 +421,7 @@ namespace Squidex.Write.Schemas sut.HideField(CreateCommand(new HideField { FieldId = 1 })); sut.ShowField(CreateCommand(new ShowField { FieldId = 1 })); - Assert.False(sut.Schema.Fields[1].IsHidden); + Assert.False(sut.Schema.FieldsById[1].IsHidden); sut.GetUncomittedEvents().Skip(1) .ShouldHaveSameEvents( @@ -469,7 +469,7 @@ namespace Squidex.Write.Schemas sut.DisableField(CreateCommand(new DisableField { FieldId = 1 })); - Assert.True(sut.Schema.Fields[1].IsDisabled); + Assert.True(sut.Schema.FieldsById[1].IsDisabled); sut.GetUncomittedEvents() .ShouldHaveSameEvents( @@ -518,7 +518,7 @@ namespace Squidex.Write.Schemas sut.DisableField(CreateCommand(new DisableField { FieldId = 1 })); sut.EnableField(CreateCommand(new EnableField { FieldId = 1 })); - Assert.False(sut.Schema.Fields[1].IsDisabled); + Assert.False(sut.Schema.FieldsById[1].IsDisabled); sut.GetUncomittedEvents().Skip(1) .ShouldHaveSameEvents( @@ -555,7 +555,7 @@ namespace Squidex.Write.Schemas sut.DeleteField(CreateCommand(new DeleteField { FieldId = 1 })); - Assert.False(sut.Schema.Fields.ContainsKey(1)); + Assert.False(sut.Schema.FieldsById.ContainsKey(1)); sut.GetUncomittedEvents() .ShouldHaveSameEvents(