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

TeardownDataOnRebuild = false prevents deleting existing projections #3213

Merged
merged 2 commits into from
May 20, 2024

Conversation

AlexZeitler
Copy link
Sponsor Contributor

It's a repro for this discussion on Discord.

@AlexZeitler
Copy link
Sponsor Contributor Author

When commenting out this line, the test succeeds.

@AlexZeitler
Copy link
Sponsor Contributor Author

Would it make sense to add TeardownDataOnRebuild to AsyncOptions and make the call to teardownExistingProjectionProgress (here) conditional based on the new property?

I've added it locally and set the option when adding the projection and my test then succeeded.

But it also broke some other tests.

@AlexZeitler
Copy link
Sponsor Contributor Author

AlexZeitler commented May 18, 2024

Digging into the history, I noticed that TeardownDataOnRebuild started to be available starting with Commit 3a9ba4e, but it already broke with the next one f7642cd.

The test I ran is similar to the one in this PR but it's using MultiStreamAggregation (before it has been refactored to MultiStreamProjection.

At first, Commit 682c9e1 looked suspicious to my because of the refactoring, but aside from completely removing the check completely it didn't make the result worse.

using System;
using System.Threading;
using System.Threading.Tasks;
using Marten.Events.Projections;
using Marten.Testing.Harness;
using Xunit;

namespace Marten.AsyncDaemon.Testing;

public class Base
{
    public string Id { get; set; }
}

public class ImplementationA: Base
{
}

public class ImplementationB: Base
{
}

public class SomethingHappened
{
    public Guid Id { get; set; }
}

public class ImplementationAProjection: MultiStreamAggregation<ImplementationA, string>
{
    public ImplementationAProjection()
    {
        Identity<SomethingHappened>(e => $"a-{e.Id}");
        TeardownDataOnRebuild = false;
    }

    public static ImplementationA Create(
        SomethingHappened e
    ) => new() { Id = $"a-{e.Id}" };
}

public class ImplementationBProjection: MultiStreamAggregation<ImplementationB, string>
{
    public ImplementationBProjection()
    {
        Identity<SomethingHappened>(e => $"b-{e.Id}");
        TeardownDataOnRebuild = false;
    }

    public static ImplementationB Create(
        SomethingHappened e
    ) => new() { Id = $"b-{e.Id}" };
}

public class MultiStreamDataTeardownOnRebuildTests: OneOffConfigurationsContext
{
    [Fact]
    public async Task Adheres_to_disabled_teardown_on_rebuild()
    {
        StoreOptions(
            opts =>
            {
                opts.Schema.For<Base>()
                    .AddSubClass<ImplementationA>()
                    .AddSubClass<ImplementationB>();

                opts.Projections.Add<ImplementationAProjection>(ProjectionLifecycle.Inline);

                opts.Projections.Add<ImplementationBProjection>(ProjectionLifecycle.Inline);
            }
        );

        var commonId = Guid.NewGuid();

        using var daemon = await theStore.BuildProjectionDaemonAsync();
        await using var session = theStore.LightweightSession();

        session.Events.StartStream(
            commonId,
            new SomethingHappened() { Id = commonId }
        );
        await session.SaveChangesAsync();

        var implementationA = await session.LoadAsync<ImplementationA>($"a-{commonId}");
        var implementationB = await session.LoadAsync<ImplementationB>($"b-{commonId}");

        implementationA.ShouldNotBeNull();
        implementationB.ShouldNotBeNull();

        await daemon.RebuildProjection<ImplementationAProjection>(CancellationToken.None);

        var implementationBAfterRebuildOfA = await session.LoadAsync<ImplementationB>($"b-{commonId}");

        implementationBAfterRebuildOfA.ShouldNotBeNull();
    }
}

@AlexZeitler
Copy link
Sponsor Contributor Author

After a few random checks in the history up to the current status, I am tempted to say that it has never worked again apart from the first commit.

@AlexZeitler
Copy link
Sponsor Contributor Author

Turns out, that the failing tests also failed without my changes.

Before applying my changes:
image

After applying my changes:
image

@AlexZeitler
Copy link
Sponsor Contributor Author

As can be seen in the tests, the fix doesn't use the existing TeardownDataOnRebuild property from ProjectionBase but Options.TeardownDataOnRebuild from AsyncOptions on IProjectionSource.

Having TeardownDataOnRebuild in AsyncOptions it can be set either in the Projection implementation or in the registration.

@AlexZeitler AlexZeitler changed the title Test: TeardownDataOnRebuild = false should prevent deleting projections TeardownDataOnRebuild = false prevents deleting existing projections May 20, 2024
@jeremydmiller jeremydmiller merged commit 4cf2f03 into JasperFx:master May 20, 2024
11 checks passed
@AlexZeitler AlexZeitler deleted the multistream-rebuild branch May 20, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants