RavenDB-14963 - All revisions view #18504

shaharhikri · 2024-05-12T11:55:39Z

Issue link

https://issues.hibernatingrhinos.com/issue/RavenDB-14963/All-revisions-view

Additional description

Add All revisions view EP

{
"TotalResults": 8,
"Results": [
{
"Id": "Users/1",
"Etag": 16,
"LastModified": "2024-05-12T11:53:41.8128727Z",
"ChangeVector": "A:15-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Companies/1",
"Etag": 14,
"LastModified": "2024-05-12T11:53:35.7369914Z",
"ChangeVector": "A:13-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Companies/1",
"Etag": 12,
"LastModified": "2024-05-12T11:53:33.9179408Z",
"ChangeVector": "A:11-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Docs/1",
"Etag": 10,
"LastModified": "2024-05-12T11:53:23.5946301Z",
"ChangeVector": "A:9-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Docs/1",
"Etag": 8,
"LastModified": "2024-05-12T11:53:21.7090247Z",
"ChangeVector": "A:7-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Users/1",
"Etag": 6,
"LastModified": "2024-05-12T11:53:13.4545503Z",
"ChangeVector": "A:5-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Users/1",
"Etag": 4,
"LastModified": "2024-05-09T08:34:28.8081143Z",
"ChangeVector": "A:3-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
},
{
"Id": "Users/1",
"Etag": 2,
"LastModified": "2024-05-09T08:34:25.7935544Z",
"ChangeVector": "A:1-UkgaAlPii063JyiBPFBdYQ",
"Flags": "HasRevisions, Revision"
}
]
}

Type of change

Bug fix
Regression bug fix
Optimization
New feature

How risky is the change?

Low
Moderate
High
Not relevant

Backward compatibility

Non breaking change
Ensured. Please explain how has it been implemented?
Breaking change
Not relevant

Is it platform specific issue?

Yes. Please list the affected platforms.
No

Documentation update

This change requires a documentation update. Please mark the issue on YouTrack using Documentation Required tag.
No documentation update is needed

Testing by Contributor

Tests have been added that prove the fix is effective or that the feature works
Internal classes added to the test class (e.g. entity or index definition classes) have the lowest possible access modifier (preferable private)
It has been verified by manual testing

Testing by RavenDB QA team

This change requires a special QA testing due to possible performance or resources usage implications (CPU, memory, IO). Please mark the issue on YouTrack using QA Required tag.
No special testing by RavenDB QA team is needed

Is there any existing behavior change of other features due to this change?

Yes. Please list the affected features/subsystems and provide appropriate explanation
No

UI work

It requires further work in the Studio. Please mark the issue on YouTrack using Studio Required tag.
No UI work is needed

karmeli87

Looks good, most of the pieces are in place.
Can we also add few tests for this?

...eb/Studio/Sharding/Processors/ShardedStudioCollectionsHandlerProcessorForPreviewRevisions.cs

src/Raven.Server/Documents/Revisions/RevisionsStorage.cs

...Server/Web/Studio/Processors/AbstractStudioCollectionsHandlerProcessorForPreviewRevisions.cs

...eb/Studio/Sharding/Processors/ShardedStudioCollectionsHandlerProcessorForPreviewRevisions.cs

src/Raven.Server/Documents/Sharding/ShardedDatabaseContext.Streaming.cs

...Server/Web/Studio/Processors/AbstractStudioCollectionsHandlerProcessorForPreviewRevisions.cs

...eb/Studio/Sharding/Processors/ShardedStudioCollectionsHandlerProcessorForPreviewRevisions.cs

karmeli87 · 2024-05-13T13:19:55Z

src/Raven.Server/Web/Studio/StudioCollectionsHandlerProcessorForPreviewRevisions.cs

+        var first = true;
+        foreach (var revision in revisions)
+        {
+            if (first)
+                first = false;
+            else
+                writer.WriteComma();
+
+            writer.WriteStartObject();
+
+            writer.WritePropertyName(nameof(Document.Id));
+            writer.WriteString(revision.Id);
+            writer.WriteComma();
+
+            writer.WritePropertyName(nameof(Document.Etag));
+            writer.WriteInteger(revision.Etag);
+            writer.WriteComma();
+
+            writer.WritePropertyName(nameof(Document.LastModified));
+            writer.WriteDateTime(revision.LastModified, true);
+            writer.WriteComma();
+
+            writer.WritePropertyName(nameof(Document.ChangeVector));
+            writer.WriteString(revision.ChangeVector);
+            writer.WriteComma();
+
+            writer.WritePropertyName(nameof(Document.Flags));
+            writer.WriteString(revision.Flags.ToString());
+
+            writer.WriteEndObject();
+        }
+
+        writer.WriteEndArray();


Would be nice we could unify this code with the sharded one...
for that I guess we will need to project the revision to something like
(Id, Etag, LastModified, ChangeVector, Flags)

handle rest of the comments first and ping about this one at the end

Talked about it and decided to pass

...Server/Web/Studio/Processors/AbstractStudioCollectionsHandlerProcessorForPreviewRevisions.cs

karmeli87 · 2024-05-15T12:53:39Z

src/Raven.Server/Documents/Sharding/Streaming/ShardResultConverter.cs

+using System;
+using System.Collections.Generic;


cannot remove the using System.Collections.Generic;

test/SlowTests/Issues/RavenDB-14963.cs

ppekrol · 2024-05-15T13:53:37Z

...Server/Web/Studio/Processors/AbstractStudioCollectionsHandlerProcessorForPreviewRevisions.cs

+            }
+
+            if (etag != null)
+                HttpContext.Response.Headers["ETag"] = "\"" + etag + "\"";


use Constants.Headers.Etag here

ppekrol · 2024-05-15T13:54:00Z

...Server/Web/Studio/Processors/AbstractStudioCollectionsHandlerProcessorForPreviewRevisions.cs

+                writer.WriteComma();
+
+                writer.WritePropertyName(nameof(PreviewRevisionsResult.Results));
+                await WriteItems(context, writer);


WriteItemsAsync

ppekrol · 2024-05-15T13:55:08Z

src/Raven.Server/Web/Studio/Sharding/ShardedStudioCollectionsHandler.cs

@@ -21,5 +22,13 @@ public async Task Delete()
            using (var processor = new ShardedStudioCollectionHandlerProcessorForDeleteCollection(this))
                await processor.ExecuteAsync();
        }
+
+        [RavenShardedAction("/databases/*/studio/revisions/preview", "GET")]
+


arekpalinski · 2024-05-17T06:35:25Z

src/Raven.Server/Documents/Revisions/RevisionsStorage.cs

+        public IEnumerable<Document> GetRevisionsInReverseEtagOrder(DocumentsOperationContext context, int skip, int take)
+        {
+            return GetRevisionsInReverseEtagOrderInternal(context, 
+                table: new Table(RevisionsSchema, context.Transaction.InnerTransaction), 


We have CompressedRevisionsSchema as well. Don't we need to distinguish here based on _database.DocumentsCompression.CompressRevisions?

#18504 (comment)

arekpalinski · 2024-05-17T06:36:39Z

src/Raven.Server/Documents/Revisions/RevisionsStorage.cs

+        public IEnumerable<Document> GetRevisionsInReverseEtagOrderForCollection(DocumentsOperationContext context, string collection, int skip, int take)
+        {
+            var collectionName = _documentsStorage.ExtractCollectionName(context, collection);
+            var table = EnsureRevisionTableCreated(context.Transaction.InnerTransaction, collectionName);


Here EnsureRevisionTableCreated does the _database.DocumentsCompression.CompressRevisions check under the covers

We should do this compression check only on write.
The CompressedRevisionsSchema and the RevisionsSchema are basically the same, except the Compressed property.
We use this property only on write.
On read we look at the page metadata, to see whether it (the page) is compressed or not.
so I reverted those changes.

karmeli87 · 2024-05-23T06:31:33Z

src/Raven.Server/Documents/Revisions/RevisionsStorage.cs

+            return table.GetNumberOfEntriesFor(RevisionsSchema.FixedSizeIndexes[CollectionRevisionsEtagsSlice]);
+        }
+
+        internal Table GetOrCreateTable(Transaction tx, CollectionName collection)


why not to use EnsureRevisionTableCreated instead ?

karmeli87 · 2024-05-23T06:36:59Z

...Server/Web/Studio/Processors/AbstractStudioCollectionsHandlerProcessorForPreviewRevisions.cs

+        {
+            Collection = RequestHandler.GetStringQueryString("collection", required: false);
+
+            if (NotModified(context, out var etag))


how can this be before InitializeAsync ? how would the sharded database know it is not modified?

github-actions bot added the v6.0 label May 12, 2024

karmeli87 requested changes May 13, 2024

View reviewed changes

karmeli87 reviewed May 13, 2024

View reviewed changes

shaharhikri force-pushed the RavenDB-14963 branch 4 times, most recently from 69f4d61 to 6fe675b Compare May 15, 2024 12:47

karmeli87 reviewed May 15, 2024

View reviewed changes

karmeli87 requested a review from ppekrol May 15, 2024 13:27

ppekrol approved these changes May 15, 2024

View reviewed changes

shaharhikri force-pushed the RavenDB-14963 branch from 22ee945 to e055e88 Compare May 15, 2024 14:05

arekpalinski reviewed May 17, 2024

View reviewed changes

shaharhikri added 7 commits May 19, 2024 11:26

RavenDB-14963 - Revisions view EP

306878c

RavenDB-14963 - Addressed PR Comments

dbbb0be

RavenDB-14963 - remove AbstractDatabaseHandlerProcessor inheritance

5d80423

RavenDB-14963 - All Revisions View Test

769136f

RavenDB-14963 - Addressed PR Comments

d3fd37e

RavenDB-14963 - inherit from AbstractHandlerProcessor

1342b71

RavenDB-14963 - Addressed PR Comments

daa599b

shaharhikri force-pushed the RavenDB-14963 branch 2 times, most recently from ca41a69 to e4bb76a Compare May 19, 2024 09:25

arekpalinski requested a review from karmeli87 May 20, 2024 06:33

shaharhikri force-pushed the RavenDB-14963 branch from e4bb76a to e13a918 Compare May 20, 2024 07:40

RavenDB-14963 - Make tests using session to get revisions

fc4554f

shaharhikri force-pushed the RavenDB-14963 branch from e13a918 to fc4554f Compare May 20, 2024 11:37

arekpalinski approved these changes May 23, 2024

View reviewed changes

karmeli87 reviewed May 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RavenDB-14963 - All revisions view #18504

RavenDB-14963 - All revisions view #18504

shaharhikri commented May 12, 2024 •

edited

karmeli87 left a comment

karmeli87 May 13, 2024

karmeli87 May 13, 2024

shaharhikri May 15, 2024

karmeli87 May 15, 2024

shaharhikri May 15, 2024

ppekrol May 15, 2024

shaharhikri May 15, 2024

ppekrol May 15, 2024

shaharhikri May 15, 2024

ppekrol May 15, 2024

shaharhikri May 15, 2024

arekpalinski May 17, 2024

shaharhikri May 19, 2024

shaharhikri May 20, 2024

arekpalinski May 17, 2024

shaharhikri May 19, 2024

shaharhikri May 20, 2024 •

edited

arekpalinski May 21, 2024

karmeli87 May 23, 2024

karmeli87 May 23, 2024

RavenDB-14963 - All revisions view #18504

Are you sure you want to change the base?

RavenDB-14963 - All revisions view #18504

Conversation

shaharhikri commented May 12, 2024 • edited

Issue link

Additional description

Type of change

How risky is the change?

Backward compatibility

Is it platform specific issue?

Documentation update

Testing by Contributor

Testing by RavenDB QA team

Is there any existing behavior change of other features due to this change?

UI work

karmeli87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaharhikri May 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaharhikri commented May 12, 2024 •

edited

shaharhikri May 20, 2024 •

edited