From fa9fbc02b6b05c7eca39e0230a31a4c48f0c43f9 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 13 Nov 2019 14:24:34 -0500 Subject: [PATCH] t5799: add support for POST to return either a loose object or packfile Earlier versions of the test always returned a packfile in response to a POST. Now we look at the number of objects in the POST request. If > 1, always send a packfile. If = 1 and it is a commit, send a packfile. Otherwise, send a loose object. This is to better model the behavior of the GVFS server/protocol which treats commits differently. Signed-off-by: Jeff Hostetler --- t/helper/test-gvfs-protocol.c | 187 +++++++++++++++++++++++----------- t/t5799-gvfs-helper.sh | 76 +++++++++++++- 2 files changed, 203 insertions(+), 60 deletions(-) diff --git a/t/helper/test-gvfs-protocol.c b/t/helper/test-gvfs-protocol.c index 1305b2c6b83a85..f4dee661eedeba 100644 --- a/t/helper/test-gvfs-protocol.c +++ b/t/helper/test-gvfs-protocol.c @@ -492,8 +492,7 @@ static enum worker_result do__gvfs_config__get(struct req *req) * write_object_file_prepare() * write_loose_object() */ -static enum worker_result send_loose_object(const struct object_info *oi, - const struct object_id *oid, +static enum worker_result send_loose_object(const struct object_id *oid, int fd) { #define MAX_HEADER_LEN 32 @@ -506,6 +505,42 @@ static enum worker_result send_loose_object(const struct object_info *oi, git_hash_ctx c; int object_header_len; int ret; + unsigned flags = 0; + void *content; + unsigned long size; + enum object_type type; + struct object_info oi = OBJECT_INFO_INIT; + + /* + * Since `test-gvfs-protocol` is mocking a real GVFS server (cache or + * main), we don't want a request for a missing object to cause the + * implicit dynamic fetch mechanism to try to fault-it-in (and cause + * our call to oid_object_info_extended() to launch another instance + * of `gvfs-helper` to magically fetch it (which would connect to a + * new instance of `test-gvfs-protocol`)). + * + * Rather, we want a missing object to fail, so we can respond with + * a 404, for example. + */ + flags |= OBJECT_INFO_FOR_PREFETCH; + flags |= OBJECT_INFO_LOOKUP_REPLACE; + + oi.typep = &type; + oi.sizep = &size; + oi.contentp = &content; + + if (oid_object_info_extended(the_repository, oid, &oi, flags)) { + logerror("Could not find OID: '%s'", oid_to_hex(oid)); + return send_http_error(1, 404, "Not Found", -1, WR_OK); + } + + if (string_list_has_string(&mayhem_list, "http_404")) { + logmayhem("http_404"); + return send_http_error(1, 404, "Not Found", -1, WR_MAYHEM); + } + + trace2_printf("%s: OBJECT type=%d len=%ld '%.40s'", TR2_CAT, + type, size, (const char *)content); /* * We are blending several somewhat independent concepts here: @@ -560,13 +595,13 @@ static enum worker_result send_loose_object(const struct object_info *oi, /* [1a] */ object_header_len = 1 + xsnprintf(object_header, MAX_HEADER_LEN, "%s %"PRIuMAX, - type_name(*oi->typep), - (uintmax_t)*oi->sizep); + type_name(*oi.typep), + (uintmax_t)*oi.sizep); /* [2] */ the_hash_algo->init_fn(&c); the_hash_algo->update_fn(&c, object_header, object_header_len); - the_hash_algo->update_fn(&c, *oi->contentp, *oi->sizep); + the_hash_algo->update_fn(&c, *oi.contentp, *oi.sizep); the_hash_algo->final_fn(oid_check.hash, &c); if (!oideq(oid, &oid_check)) BUG("send_loose_object[2]: invalid construction '%s' '%s'", @@ -586,8 +621,8 @@ static enum worker_result send_loose_object(const struct object_info *oi, the_hash_algo->update_fn(&c, object_header, object_header_len); /* [3, 1b, 5, 6] */ - stream.next_in = *oi->contentp; - stream.avail_in = *oi->sizep; + stream.next_in = *oi.contentp; + stream.avail_in = *oi.sizep; do { enum worker_result wr; unsigned char *in0 = stream.next_in; @@ -632,25 +667,6 @@ static enum worker_result send_loose_object(const struct object_info *oi, static enum worker_result do__gvfs_objects__get(struct req *req) { struct object_id oid; - void *content; - unsigned long size; - enum object_type type; - struct object_info oi = OBJECT_INFO_INIT; - unsigned flags = 0; - - /* - * Since `test-gvfs-protocol` is mocking a real GVFS server (cache or - * main), we don't want a request for a missing object to cause the - * implicit dynamic fetch mechanism to try to fault-it-in (and cause - * our call to oid_object_info_extended() to launch another instance - * of `gvfs-helper` to magically fetch it (which would connect to a - * new instance of `test-gvfs-protocol`)). - * - * Rather, we want a missing object to fail, so we can respond with - * a 404, for example. - */ - flags |= OBJECT_INFO_FOR_PREFETCH; - flags |= OBJECT_INFO_LOOKUP_REPLACE; if (!req->slash_args.len || get_oid_hex(req->slash_args.buf, &oid)) { @@ -661,29 +677,13 @@ static enum worker_result do__gvfs_objects__get(struct req *req) trace2_printf("%s: GET %s", TR2_CAT, oid_to_hex(&oid)); - oi.typep = &type; - oi.sizep = &size; - oi.contentp = &content; - - if (oid_object_info_extended(the_repository, &oid, &oi, flags)) { - logerror("Could not find OID: '%s'", oid_to_hex(&oid)); - return send_http_error(1, 404, "Not Found", -1, WR_OK); - } - - if (string_list_has_string(&mayhem_list, "http_404")) { - logmayhem("http_404"); - return send_http_error(1, 404, "Not Found", -1, WR_MAYHEM); - } - - trace2_printf("%s: OBJECT type=%d len=%ld '%.40s'", TR2_CAT, - type, size, (const char *)content); - - return send_loose_object(&oi, &oid, 1); + return send_loose_object(&oid, 1); } static enum worker_result read_json_post_body( struct req *req, - struct oidset *oids) + struct oidset *oids, + int *nr_oids) { struct object_id oid; struct string_list_item *item; @@ -752,7 +752,8 @@ static enum worker_result read_json_post_body( if (get_oid_hex(pstart, &oid)) goto could_not_parse_json; - oidset_insert(oids, &oid); + if (!oidset_insert(oids, &oid)) + *nr_oids += 1; trace2_printf("%s: POST %s", TR2_CAT, oid_to_hex(&oid)); /* Eat trailing whitespace after trailing DQUOTE */ @@ -796,16 +797,6 @@ static enum worker_result read_json_post_body( * * My assumption here is that we're not testing with GBs * of data.... - * - * Note: The GVFS Protocol POST verb behaves like GET for - * Note: non-commit objects (in that it just returns the - * Note: requested object), but for commit objects POST - * Note: *also* returns all trees referenced by the commit. - * Note: - * Note: Since the goal of this test is to confirm that - * Note: gvfs-helper can request and receive a packfile - * Note: *at all*, I'm not going to blur the issue and - * Note: support the extra semantics for commit objects. */ static enum worker_result get_packfile_from_oids( struct oidset *oids, @@ -895,21 +886,99 @@ static enum worker_result send_packfile_from_buffer(const struct strbuf *packfil return wr; } +/* + * The GVFS Protocol POST verb behaves like GET for non-commit objects + * (in that it just returns the requested object), but for commit + * objects POST *also* returns all trees referenced by the commit. + * + * The goal of this test is to confirm that: + * [] `gvfs-helper post` can request and receive a packfile at all. + * [] `gvfs-helper post` can handle getting either a packfile or a + * loose object. + * + * Therefore, I'm not going to blur the issue and support the custom + * semantics for commit objects. + * + * If one of the OIDs is a commit, `git pack-objects` will completely + * walk the trees and blobs for it and we get that for free. This is + * good enough for our testing. + * + * TODO A proper solution would separate the commit objects and do a + * TODO `rev-list --filter=blobs:none` for them (or use the internal + * TODO list-objects API) and a regular enumeration for the non-commit + * TODO objects. And build an new oidset with union of those and then + * TODO call pack-objects on it instead. + * TODO + * TODO But that's too much trouble for now. + * + * For now, we just need to know if the post asks for a single object, + * is it a commit or non-commit. That is sufficient to know whether + * we should send a packfile or loose object. +*/ +static enum worker_result classify_oids_in_post( + struct oidset *oids, int nr_oids, int *need_packfile) +{ + struct oidset_iter iter; + struct object_id *oid; + enum object_type type; + struct object_info oi = OBJECT_INFO_INIT; + unsigned flags = 0; + + if (nr_oids > 1) { + *need_packfile = 1; + return WR_OK; + } + + /* disable missing-object faulting */ + flags |= OBJECT_INFO_FOR_PREFETCH; + flags |= OBJECT_INFO_LOOKUP_REPLACE; + + oi.typep = &type; + + oidset_iter_init(oids, &iter); + while ((oid = oidset_iter_next(&iter))) { + if (!oid_object_info_extended(the_repository, oid, &oi, flags) && + type == OBJ_COMMIT) { + *need_packfile = 1; + return WR_OK; + } + } + + *need_packfile = 0; + return WR_OK; +} + static enum worker_result do__gvfs_objects__post(struct req *req) { struct oidset oids = OIDSET_INIT; struct strbuf packfile = STRBUF_INIT; enum worker_result wr; + int nr_oids = 0; + int need_packfile = 0; - wr = read_json_post_body(req, &oids); + wr = read_json_post_body(req, &oids, &nr_oids); if (wr & WR_STOP_THE_MUSIC) goto done; - wr = get_packfile_from_oids(&oids, &packfile); + wr = classify_oids_in_post(&oids, nr_oids, &need_packfile); if (wr & WR_STOP_THE_MUSIC) goto done; - wr = send_packfile_from_buffer(&packfile); + if (!need_packfile) { + struct oidset_iter iter; + struct object_id *oid; + + oidset_iter_init(&oids, &iter); + oid = oidset_iter_next(&iter); + + wr = send_loose_object(oid, 1); + } else { + wr = get_packfile_from_oids(&oids, &packfile); + if (wr & WR_STOP_THE_MUSIC) + goto done; + + wr = send_packfile_from_buffer(&packfile); + } done: oidset_clear(&oids); diff --git a/t/t5799-gvfs-helper.sh b/t/t5799-gvfs-helper.sh index 437f4907fb0992..50a3d9b1822ad3 100755 --- a/t/t5799-gvfs-helper.sh +++ b/t/t5799-gvfs-helper.sh @@ -59,6 +59,7 @@ OIDS_FILE="$PWD"/oid_list.txt OIDS_CT_FILE="$PWD"/oid_ct_list.txt OIDS_BLOBS_FILE="$PWD"/oids_blobs_file.txt OID_ONE_BLOB_FILE="$PWD"/oid_one_blob_file.txt +OID_ONE_COMMIT_FILE="$PWD"/oid_one_commit_file.txt # Get a list of available OIDs in repo_src so that we can try to fetch # them and so that we don't have to hard-code a list of known OIDs. @@ -102,6 +103,11 @@ get_list_of_commit_and_tree_oids () { return 0 } +get_one_commit_oid () { + git -C "$REPO_SRC" rev-parse HEAD >"$OID_ONE_COMMIT_FILE" + return 0 +} + test_expect_success 'setup repos' ' test_create_repo "$REPO_SRC" && git -C "$REPO_SRC" branch -M main && @@ -161,7 +167,8 @@ test_expect_success 'setup repos' ' # get_list_of_oids 30 && get_list_of_commit_and_tree_oids 30 && - get_list_of_blobs_oids + get_list_of_blobs_oids && + get_one_commit_oid ' stop_gvfs_protocol_server () { @@ -511,6 +518,73 @@ test_expect_success 'basic: POST origin blobs' ' verify_connection_count 1 ' +# Request a single blob via POST. Per the GVFS Protocol, the server +# should implicitly send a loose object for it. Confirm that. +# +test_expect_success 'basic: POST-request a single blob' ' + test_when_finished "per_test_cleanup" && + start_gvfs_protocol_server && + + # Connect to the origin server (w/o auth) and request a single + # blob via POST. + # + git -C "$REPO_T1" gvfs-helper \ + --cache-server=disable \ + --remote=origin \ + --no-progress \ + post \ + <"$OID_ONE_BLOB_FILE" >OUT.output && + + # Stop the server to prevent the verification steps from faulting-in + # any missing objects. + # + stop_gvfs_protocol_server && + + # gvfs-helper prints a "loose " message for each received + # loose object. + # + sed "s/loose //" OUT.actual && + test_cmp "$OID_ONE_BLOB_FILE" OUT.actual && + + verify_connection_count 1 +' + +# Request a single commit via POST. Per the GVFS Protocol, the server +# should implicitly send us a packfile containing the commit and the +# trees it references. Confirm that properly handled the receipt of +# the packfile. (Here, we are testing that asking for a single object +# yields a packfile rather than a loose object.) +# +# We DO NOT verify that the packfile contains commits/trees and no blobs +# because our test helper doesn't implement the filtering. +# +test_expect_success 'basic: POST-request a single commit' ' + test_when_finished "per_test_cleanup" && + start_gvfs_protocol_server && + + # Connect to the origin server (w/o auth) and request a single + # commit via POST. + # + git -C "$REPO_T1" gvfs-helper \ + --cache-server=disable \ + --remote=origin \ + --no-progress \ + post \ + <"$OID_ONE_COMMIT_FILE" >OUT.output && + + # Stop the server to prevent the verification steps from faulting-in + # any missing objects. + # + stop_gvfs_protocol_server && + + # gvfs-helper prints a "packfile " message for each received + # packfile. + # + verify_received_packfile_count 1 && + + verify_connection_count 1 +' + ################################################################# # Tests to see how gvfs-helper responds to network problems. #