Skip to content

Commit

Permalink
types: fix crash when extending a type graph
Browse files Browse the repository at this point in the history
This is a workaround-quality fix. A more robust solution would involve constructor changes in every model type. Base types would need to enter through the "wrap existing" constructor instead of the "extends" constructor.
  • Loading branch information
warriordog committed Dec 14, 2023
1 parent 69443a5 commit f304216
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 11 deletions.
19 changes: 8 additions & 11 deletions Source/ActivityPub.Types/TypeMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ public TObject AsModel<TObject>()
/// <summary>
/// Extends the TypeGraph to include a new entity.
/// The entity must have a parameterless constructor.
/// Throws an exception if the entity would be a duplicate or have unmet dependencies.
/// If the entity already exists in the graph, then the existing instance is re-used.
/// Throws an exception if the entity would have unmet dependencies.
/// </summary>
/// <remarks>
/// This function cannot accomodate entities with required members.
/// Instead, <see cref="Extend{TEntity}(TEntity)"/> must be used.
/// </remarks>
/// <exception cref="InvalidOperationException">If the entity type already exists in the graph</exception>
/// <exception cref="InvalidOperationException">If the entity requires another entity that is missing from the graph</exception>
/// <see cref="Extend{TEntity}(TEntity)"/>
/// <seealso cref="Extend{TEntity}(TEntity)"/>
/// <seealso cref="AddEntity"/>
/// <seealso cref="TryAddEntity"/>
public TEntity Extend<TEntity>()
Expand All @@ -204,21 +204,18 @@ public TEntity Extend<TEntity>()

/// <summary>
/// Extends the TypeGraph to include a new entity.
/// Throws an exception if the entity would be a duplicate or have unmet dependencies.
/// If the entity already exists in the graph, then the existing instance is re-used.
/// Throws an exception if the entity would have unmet dependencies.
/// </summary>
/// <param name="entity"></param>
/// <typeparam name="TEntity"></typeparam>
/// <returns></returns>
/// <exception cref="InvalidOperationException">If the entity type already exists in the graph</exception>
/// <exception cref="InvalidOperationException">If the entity requires another entity that is missing from the graph</exception>
/// <see cref="Extend{TEntity}()"/>
/// <seealso cref="Extend{TEntity}()"/>
/// <seealso cref="AddEntity"/>
/// <seealso cref="TryAddEntity"/>
public TEntity Extend<TEntity>(TEntity entity)
where TEntity : ASEntity
{
if (IsEntity<TEntity>())
throw new InvalidOperationException($"Cannot extend the graph with entity {typeof(TEntity)}: that type already exists in the graph");
if (IsEntity<TEntity>(out var existingEntity))
return existingEntity;

// Check dependencies - this is a workaround to avoid the risk case described in TryAdd().
if (entity.BaseTypeName != null && !AllASTypes.Contains(entity.BaseTypeName))
Expand Down
52 changes: 52 additions & 0 deletions Test/ActivityPub.Types.Tests/Unit/AS/APActorTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0.
// If a copy of the MPL was not distributed with this file, You can obtain one at https://mozilla.org/MPL/2.0/.

using ActivityPub.Types.AS;
using ActivityPub.Types.AS.Extended.Actor;
using ActivityPub.Types.AS.Extended.Object;

namespace ActivityPub.Types.Tests.Unit.AS;

public abstract class APActorTests
{
public class ExtendConstructorShould : APActorTests
{
[Fact]
public void ExtendRegularObject()
{
var note = new NoteObject()
{
Id = "https://example.com/object",
Content = "content"
};

var actor = new APActor(note)
{
Inbox = "https://example.com/inbox",
Outbox = "https://example.com/outbox"
};

actor.Is<NoteObject>().Should().BeTrue();
note.Is<APActor>().Should().BeTrue();
}

[Fact]
public void NoOpWhenWrappingAnotherActor()
{
var person = new PersonActor()
{
Inbox = "https://example.com/inbox",
Outbox = "https://example.com/outbox"
};
var actor = new APActor(person)
{
Inbox = person.Inbox,
Outbox = person.Outbox
};

person.Inbox = "https://example.com/changed";

actor.Inbox.Should().Be(person.Inbox);
}
}
}

0 comments on commit f304216

Please sign in to comment.