feat: Serve current documents #32

n-dusan · 2024-01-02T20:52:19Z

Closes #18
Closes #29
Closes #31

This PR adds support for:

Serving current blobs (documents) for git repositories. We use git HEAD commit to get current blob.
Multi-host serve for git repositories based on guarded header value.

Support for serving current blobs for git repositories.

Add stelae serve cli command, which spins up the server and serves documents from Stele data repositories in an archive.

First we parse an archive and initialize Actix routes to git repositories given the Stele that's specified in config.toml. Note that Actix routes need to be registered dynamically, because the routing matching patterns depend on and are read from an external configuration file repositories.json.
For parsing, read and map all data repositories from repositories.json in Stele's authentication repo. Here's an example of a repositories.json file that expects to serve only two data repositories: open-law-html and law-static-assets.
Stele can reference other Stele in dependencies.json. To serve current blobs on referenced Stele, referenced Stele needs to specify top-level scopes in repositories.json.
After the archive is parsed, map and initialize all data git repositories to global, read-only AppState.

For testing framework, add tests for three possible archives for serving current documents are basic, multijursdiction, multihost.
Basic archive is an archive with one stele that has multiple data repositories. Multijursdiction archive is an example of Stele referencing other Stele with dependencies.json. Multihost archive is enabled by using the current_documents_guard in config.toml

Multi-host serve for git repositories based on guarded header value.

When config.toml sets a current_documents_guard header, we expect the HTTP requests to Stele to contain a guarded header. So requests which don't match the expected guarded header aren't resolved.

Not sure what I'm doing wrong here. I'm trying to make sure that all Stele are owned by parent Archive struct. But this is not working.

only partially working. I am not able to clone the git2::Repository instance, so I'm instantiating within route loop. This should not be necessary

Since Actix routes get initialized in the way they are specified, we need to sort repositories by routes. To make this work, we need a way to serialize HashMap of repository values into a Vector of tuples. This is necessary because the HashMap does not preserve order. I thought that simply serializing into BTreeMap instead would work, however, that data structure preserves order by sorting keys. What we instead want is to sort the collection by values (more specifically routes within the repository custom attribute). Based on a comment on stackoverflow [1], it seems highly unlikely we can sort BTreeMap by values. Instead, we implement our own serde deserializing method that deserializes a serde Map into a Vec<(String,Repository)> using deserialize_with [2]. [1] - https://stackoverflow.com/questions/41220872/how-if-possible-to-sort-a-btreemap-by-value-in-rust [2] - https://serde.rs/field-attrs.html#deserialize_with

…stele

Introduce a --individual flag. If wish to serve an individual stele, use the bool flag, else we expect config.toml to contain the root stele for the archive.

Made more to have name accessable from the struct, as opposed to a tuple

tombh

Looking good! You're clearly a Rust dev now Dušan 😏

It's great seeing all the tests.

I mention it a couple of times in the code review comments, but more generally I have a question about whether the tested Git repos' activity has to be included in the main repo? What I mean is that I see various diffs in this PR referring to Git hooks, Git logs, etc. Can those changes not be made in the moment of the test? Allowing it all to be Git ignored?

src/stelae/archive.rs

src/stelae/stele.rs

src/stelae/types/repositories.rs

src/utils/archive.rs

tests/archive_testtools/config.rs

tombh · 2024-01-22T11:39:13Z

tests/archive_testtools/config.rs

+/// Information about a data repository.
+///
+/// This struct is used to initialize a data repository in the test suite.
+pub struct TestDataRepositoryContext<'repo> {


I'm a bit surprised that lifetimes would be needed in tests. Though I wouldn't worry too much, if it's the easiest way to solve something then it's not a big deal, the tests hopefully don't dictate the design of the actual code.

Lifetimes can get really complicated, I'm just curious if they're really needed in test code. Is there some other "hack" to stop having to think about lifetimes in tests?

I think you're right. We don't need to optimize testing framework by using string slices instead of owned Strings. Refactored and removed lifetimes

tombh · 2024-01-22T11:58:07Z

tests/archive_testtools/config.rs

+    /// The name of the data repository.
+    pub name: &'repo str,
+    /// The paths of the data repository.
+    pub paths: Vec<Cow<'static, str>>,


I think paths should be Vec<PathBuf> to match the type of Stele.archive_path. Which makes me wonder if other types in this struct need updating to match the application's underlying type.

What tips me off is that it should be rare that tests need special treatment like lifetimes and Cow.

Changed to Vec<PathBuf>, removed lifetimes and Cow

tombh · 2024-01-22T12:02:44Z

tests/fixtures/basic/archive/test/law-html/hooks/fsmonitor-watchman.sample

@@ -0,0 +1,174 @@
+#!/usr/bin/perl


I imagine that none of these Git hooks are ever used? I wonder if they could just be deleted? Or ignored?

tombh · 2024-01-22T12:03:40Z

tests/fixtures/basic/archive/test/law-html/logs/HEAD

@@ -0,0 +1,10 @@
+0000000000000000000000000000000000000000 64215d0a7b9398be24035d95d574f329fd3140f5 n-dusan <nikolic.dusan.dey@gmail.com> 1686250251 +0200	commit (initial): Initial commit


Can such Git activity be Git-ignored?

Definitely. Added logs and hooks to gitignore

Nextest is a next-gen blessed test-runner. For local testing, it's necessary to download the nextest binary from their official releases

nextest currently does not run documenation tests, see this [1] issue. [1] - nextest-rs/nextest#16

It was essentially premature optimization

n-dusan · 2024-01-26T15:20:22Z

Thanks, @tombh! I agree, definitely didn't need to have those Git hooks and logs, etc. I believe I've addressed all comments, so re-requested review.

tombh · 2024-01-28T14:27:45Z

src/stelae/types/repositories.rs

+/// `repositories.json` is expected to exist in /targets/repositories.json in the authentication repository.
+/// # Examples
+///
+/// ```rust


tombh · 2024-01-28T14:29:32Z

tests/archive_testtools/config.rs

+            "./a/b/c.html",
+            "./a/b/c/index.html",
+        ];
+        paths.iter().map(|&x| PathBuf::from(x)).collect()


Ohhh yes, nice!

dgreisen and others added 30 commits January 1, 2024 16:36

refact: library -> archive

8aba36c

feat(wip): work towards serving current documents

a102e04

feat(wip): Archive and Stele structs

aa3b773

Not sure what I'm doing wrong here. I'm trying to make sure that all Stele are owned by parent Archive struct. But this is not working.

refact(wip): turn stele backreference into path

25e00ac

feat(wip): load dependencies using serde json

9ec11d0

feat(wip): naive traversal

dfd18ab

feat: recursively load dependencies.json from stele'

47dbcf0

chore: remove unused test suite

d9fd153

fix: optionally load from repositories.json

3a55eaf

feat(wip): progress towards dynamic routes and dynamic appdata

11a7d07

feat(wip): set up repository state to each route and scope

f38242b

only partially working. I am not able to clone the git2::Repository instance, so I'm instantiating within route loop. This should not be necessary

wip: progress towards serving blobs

ddc63f0

feat(wip): serve current blobs

5cb8840

feat(wip): add underscore routes to parent scope

31a4919

wip: work on setting root stele

adc335b

wip: rename variables

f54a674

wip: underscore routes

7aedda4

refact: reuse new associated function for both root and individual …

1ea8123

…stele

feat: simplify getting the root stele

66e8bdb

Introduce a --individual flag. If wish to serve an individual stele, use the bool flag, else we expect config.toml to contain the root stele for the archive.

wip: simplify pathing to stele

22f7bee

wip: fallback

e80be4e

chore: formatting

82aa032

feat: add util function for extracting {org}/{name}

fa2c3bc

refact: move name to Repository struct

9a80b8e

Made more to have name accessable from the struct, as opposed to a tuple

feat: retrieve fallback repository

27c4dc9

wip: shared fallback repository is initialized

8cad7b9

fix: serve fallback

c541033

fix: rename dynamic segments and confirm that _ routing works

327d53c

chore: use .stelae instead of .stele in testing

f871424

n-dusan added the enhancement New feature or request label Jan 2, 2024

n-dusan self-assigned this Jan 2, 2024

fix: delete unwanted .vscode settings, update .gitignore

9e30f5f

n-dusan force-pushed the ndusan/serve-current-documents branch from b3cfca9 to 557b65f Compare January 2, 2024 21:24

fix: remove unwanted fixtures

ab634da

n-dusan force-pushed the ndusan/serve-current-documents branch from 557b65f to ab634da Compare January 2, 2024 21:37

n-dusan mentioned this pull request Jan 3, 2024

Dgreisen/stelae #10

Closed

n-dusan requested a review from tombh January 3, 2024 14:04

tombh reviewed Jan 22, 2024

View reviewed changes

n-dusan added 13 commits January 24, 2024 18:25

refact: use format string interpolation

14ad04a

refact: use into instead of to_owned for Strings

f6598b6

refact: separate json deserialization visitors into own functions

ee1656a

fixup: add docstrings

be9303c

refact: use nextest for ci testing

42823ac

Nextest is a next-gen blessed test-runner. For local testing, it's necessary to download the nextest binary from their official releases

chore: remove typo

b70680a

refact: add documentation test for json serialization

08cd60a

refact: update nextest to also run doc tests

a6ac3f8

nextest currently does not run documenation tests, see this [1] issue. [1] - nextest-rs/nextest#16

refact(test): convert default paths str into owned vec elements

3588b48

refact(test): avoid unnecessary lifetimes and cow

aa4168b

It was essentially premature optimization

refact: rename parameter for fix_unc_path fn

1086f98

refact(test): use is_none instead of if let None`

801ec43

refact(test): use PathBuf for paths instead of Strings

88dc3c8

n-dusan force-pushed the ndusan/serve-current-documents branch from ad826db to 88dc3c8 Compare January 26, 2024 14:43

n-dusan added 2 commits January 26, 2024 15:49

fix(test): untrack git logs and hooks, add gitignore

cf8cf6e

fix(bench): fix git benchmark

fd3a3ed

n-dusan requested a review from tombh January 26, 2024 15:19

tombh approved these changes Jan 28, 2024

View reviewed changes

n-dusan merged commit 9aaed03 into main Jan 29, 2024
4 checks passed

n-dusan deleted the ndusan/serve-current-documents branch January 29, 2024 14:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Serve current documents #32

feat: Serve current documents #32

n-dusan commented Jan 2, 2024

tombh left a comment

tombh Jan 22, 2024

n-dusan Jan 26, 2024

tombh Jan 22, 2024

n-dusan Jan 26, 2024

tombh Jan 22, 2024

tombh Jan 22, 2024

n-dusan Jan 26, 2024 •

edited

Loading

n-dusan commented Jan 26, 2024

tombh Jan 28, 2024

tombh Jan 28, 2024

		@@ -0,0 +1,10 @@
		0000000000000000000000000000000000000000 64215d0a7b9398be24035d95d574f329fd3140f5 n-dusan <nikolic.dusan.dey@gmail.com> 1686250251 +0200 commit (initial): Initial commit

feat: Serve current documents #32

feat: Serve current documents #32

Conversation

n-dusan commented Jan 2, 2024

Support for serving current blobs for git repositories.

Multi-host serve for git repositories based on guarded header value.

tombh 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

n-dusan Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

n-dusan commented Jan 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n-dusan Jan 26, 2024 •

edited

Loading