Marten7 UseNumericRevisions(false) keeps m.Metadata.Revision.Enabled enabled for Async projections

[murlakatam]

Seems like Marten7 is forcing numeric revisions on async projections by this policy (it was definitely a surprise during upgrade and should have been mentioned in the Migration Guide and not only on the concurency page)

Is there a reason why numeric is forcefully enabled in this case?
Seems like the policy should have checked if IVersioned with Guid was present on the projection type?
Perhaps there should be a way to opt out for Guid Revision?

The problem is when you try to opt out from this via explicit UseNumericRevisions(false),

options.Projections.Snapshot<SomeProjectionAggregate>(SnapshotLifecycle.Async)
            .UseNumericRevisions(false);

then m.Metadata.Revision.Enabled = true is still enabled resulting in exception during schema patch generation

Unhandled exception: System.ArgumentException: An item with the same key has already been added. Key: mt_version
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey](List`1 source, Func`2 keySelector, IEqualityComparer`1 comparer)
   at Weasel.Postgresql.Tables.ItemDelta`1..ctor(IEnumerable`1 expectedItems, IEnumerable`1 actualItems, Func`3 comparison)
   at Weasel.Postgresql.Tables.TableDelta.compare(Table expected, Table actual)
   at Weasel.Core.SchemaObjectDelta`1..ctor(T expected, T actual)
   at Weasel.Postgresql.Tables.TableDelta..ctor(Table expected, Table actual)
   at Weasel.Postgresql.Tables.Table.CreateDeltaAsync(DbDataReader reader, CancellationToken ct)
   at Weasel.Core.SchemaMigration.DetermineAsync(DbConnection conn, CancellationToken ct, ISchemaObject[] schemaObjects)
   at Weasel.Core.SchemaMigration.DetermineAsync(DbConnection conn, CancellationToken ct, ISchemaObject[] schemaObjects)

/// <summary>
      /// Directs Marten to use the numeric revisioning for this specific
      /// document type
      /// </summary>
      /// <returns></returns>
      public MartenRegistry.DocumentMappingExpression<T> UseNumericRevisions(bool enabled)
      {
        this._builder.Alter = (Action<DocumentMapping<T>>) (m =>
        {
          m.UseNumericRevisions = enabled;
          if (!enabled)  // the method will return here
            return;
          m.Metadata.Revision.Enabled = true; // this revision would be kept enabled. 
        });
        return this;
      }

IMO we need to either disable as well via m.Metadata.Revision.Enabled = enabled inside UseNumericRevisions(false) or provide an alternative extension method to do switch to Guid based revisions

[jeremydmiller]

@murlakatam

"Is there a reason why numeric is forcefully enabled in this case?
Seems like the policy should have checked if IVersioned with Guid was present on the projection type?
Perhaps there should be a way to opt out for Guid Revision?"

1. Yes, the numeric revision was necessary to enable the blue/green deployments
2. Yes, I missed this in the migration guide out of the sheer number of things that were going on. I apologize. Also, "I take pull requests"
3. It never made any sense to use optimistic concurrency on a document type that was being built by projections and I've always recommended against doing that because of the risk of destabilizing the async daemon

[jeremydmiller]

@murlakatam I guess I'd like to hear your argument for why it makes any sense at all to use the Guid based versioning for a projected document. At this moment, I'm not enthusiastic about making any changes here. The revisioning is 100% necessary for this:

https://jeremydmiller.com/2024/03/05/marten-7-makes-write-model-projections-super/

[jeremydmiller]

The user's problem here would be addressed by other fixes