diff --git a/include/git2/sys/odb_backend.h b/include/git2/sys/odb_backend.h index 31ffe1c33..4365906d4 100644 --- a/include/git2/sys/odb_backend.h +++ b/include/git2/sys/odb_backend.h @@ -64,6 +64,16 @@ struct git_odb_backend { int (* exists)( git_odb_backend *, const git_oid *); + /** + * If the backend implements a refreshing mechanism, it should be exposed + * through this endpoint. Each call to `git_odb_refresh()` will invoke it. + * + * However, the backend implementation should try to stay up-to-date as much + * as possible by itself as libgit2 will not automatically invoke + * `git_odb_refresh()`. For instance, a potential strategy for the backend + * implementation to achieve this could be to internally invoke this + * endpoint on failed lookups (ie. `exists()`, `read()`, `read_header()`). + */ int (* refresh)(git_odb_backend *); int (* foreach)( diff --git a/src/odb.c b/src/odb.c index 21b46bf56..e47715f79 100644 --- a/src/odb.c +++ b/src/odb.c @@ -608,7 +608,6 @@ int git_odb_exists(git_odb *db, const git_oid *id) git_odb_object *object; size_t i; bool found = false; - bool refreshed = false; assert(db && id); @@ -617,7 +616,6 @@ int git_odb_exists(git_odb *db, const git_oid *id) return (int)true; } -attempt_lookup: for (i = 0; i < db->backends.length && !found; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; @@ -626,16 +624,6 @@ attempt_lookup: found = b->exists(b, id); } - if (!found && !refreshed) { - if (git_odb_refresh(db) < 0) { - giterr_clear(); - return (int)false; - } - - refreshed = true; - goto attempt_lookup; - } - return (int)found; } @@ -700,7 +688,6 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) { size_t i, reads = 0; int error; - bool refreshed = false; git_rawobj raw; git_odb_object *object; @@ -710,7 +697,6 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) if (*out != NULL) return 0; -attempt_lookup: error = GIT_ENOTFOUND; for (i = 0; i < db->backends.length && error < 0; ++i) { @@ -723,14 +709,6 @@ attempt_lookup: } } - if (error == GIT_ENOTFOUND && !refreshed) { - if ((error = git_odb_refresh(db)) < 0) - return error; - - refreshed = true; - goto attempt_lookup; - } - if (error && error != GIT_PASSTHROUGH) { if (!reads) return git_odb__error_notfound("no match for id", id); @@ -752,7 +730,7 @@ int git_odb_read_prefix( git_oid found_full_oid = {{0}}; git_rawobj raw; void *data = NULL; - bool found = false, refreshed = false; + bool found = false; git_odb_object *object; assert(out && db); @@ -769,7 +747,6 @@ int git_odb_read_prefix( return 0; } -attempt_lookup: for (i = 0; i < db->backends.length; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; @@ -796,14 +773,6 @@ attempt_lookup: } } - if (!found && !refreshed) { - if ((error = git_odb_refresh(db)) < 0) - return error; - - refreshed = true; - goto attempt_lookup; - } - if (!found) return git_odb__error_notfound("no match for prefix", short_id); diff --git a/src/odb_pack.c b/src/odb_pack.c index 43880612a..d24b4aa99 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -342,8 +342,9 @@ static int pack_backend__refresh(git_odb_backend *_backend) return 0; } - -static int pack_backend__read_header(size_t *len_p, git_otype *type_p, struct git_odb_backend *backend, const git_oid *oid) +static int pack_backend__read_header_internal( + size_t *len_p, git_otype *type_p, + struct git_odb_backend *backend, const git_oid *oid) { struct git_pack_entry e; int error; @@ -356,7 +357,26 @@ static int pack_backend__read_header(size_t *len_p, git_otype *type_p, struct gi return git_packfile_resolve_header(len_p, type_p, e.p, e.offset); } -static int pack_backend__read(void **buffer_p, size_t *len_p, git_otype *type_p, git_odb_backend *backend, const git_oid *oid) +static int pack_backend__read_header( + size_t *len_p, git_otype *type_p, + struct git_odb_backend *backend, const git_oid *oid) +{ + int error; + + error = pack_backend__read_header_internal(len_p, type_p, backend, oid); + + if (error != GIT_ENOTFOUND) + return error; + + if ((error = pack_backend__refresh(backend)) < 0) + return error; + + return pack_backend__read_header_internal(len_p, type_p, backend, oid); +} + +static int pack_backend__read_internal( + void **buffer_p, size_t *len_p, git_otype *type_p, + git_odb_backend *backend, const git_oid *oid) { struct git_pack_entry e; git_rawobj raw; @@ -373,7 +393,24 @@ static int pack_backend__read(void **buffer_p, size_t *len_p, git_otype *type_p, return 0; } -static int pack_backend__read_prefix( +static int pack_backend__read( + void **buffer_p, size_t *len_p, git_otype *type_p, + git_odb_backend *backend, const git_oid *oid) +{ + int error; + + error = pack_backend__read_internal(buffer_p, len_p, type_p, backend, oid); + + if (error != GIT_ENOTFOUND) + return error; + + if ((error = pack_backend__refresh(backend)) < 0) + return error; + + return pack_backend__read_internal(buffer_p, len_p, type_p, backend, oid); +} + +static int pack_backend__read_prefix_internal( git_oid *out_oid, void **buffer_p, size_t *len_p, @@ -410,9 +447,45 @@ static int pack_backend__read_prefix( return error; } +static int pack_backend__read_prefix( + git_oid *out_oid, + void **buffer_p, + size_t *len_p, + git_otype *type_p, + git_odb_backend *backend, + const git_oid *short_oid, + size_t len) +{ + int error; + + error = pack_backend__read_prefix_internal( + out_oid, buffer_p, len_p, type_p, backend, short_oid, len); + + if (error != GIT_ENOTFOUND) + return error; + + if ((error = pack_backend__refresh(backend)) < 0) + return error; + + return pack_backend__read_prefix_internal( + out_oid, buffer_p, len_p, type_p, backend, short_oid, len); +} + static int pack_backend__exists(git_odb_backend *backend, const git_oid *oid) { struct git_pack_entry e; + int error; + + error = pack_entry_find(&e, (struct pack_backend *)backend, oid); + + if (error != GIT_ENOTFOUND) + return error == 0; + + if ((error = pack_backend__refresh(backend)) < 0) { + giterr_clear(); + return (int)false; + } + return pack_entry_find(&e, (struct pack_backend *)backend, oid) == 0; } diff --git a/tests-clar/odb/backend/nonrefreshing.c b/tests-clar/odb/backend/nonrefreshing.c new file mode 100644 index 000000000..abb824d4b --- /dev/null +++ b/tests-clar/odb/backend/nonrefreshing.c @@ -0,0 +1,261 @@ +#include "clar_libgit2.h" +#include "git2/sys/odb_backend.h" +#include "repository.h" + +typedef struct fake_backend { + git_odb_backend parent; + + git_error_code error_code; + + int exists_calls; + int read_calls; + int read_header_calls; + int read_prefix_calls; +} fake_backend; + +static git_repository *_repo; +static fake_backend *_fake; +static git_oid _oid; + +static int fake_backend__exists(git_odb_backend *backend, const git_oid *oid) +{ + fake_backend *fake; + + GIT_UNUSED(oid); + + fake = (fake_backend *)backend; + + fake->exists_calls++; + + return (fake->error_code == GIT_OK); +} + +static int fake_backend__read( + void **buffer_p, size_t *len_p, git_otype *type_p, + git_odb_backend *backend, const git_oid *oid) +{ + fake_backend *fake; + + GIT_UNUSED(buffer_p); + GIT_UNUSED(len_p); + GIT_UNUSED(type_p); + GIT_UNUSED(oid); + + fake = (fake_backend *)backend; + + fake->read_calls++; + + *len_p = 0; + *buffer_p = NULL; + *type_p = GIT_OBJ_BLOB; + + return fake->error_code; +} + +static int fake_backend__read_header( + size_t *len_p, git_otype *type_p, + git_odb_backend *backend, const git_oid *oid) +{ + fake_backend *fake; + + GIT_UNUSED(len_p); + GIT_UNUSED(type_p); + GIT_UNUSED(oid); + + fake = (fake_backend *)backend; + + fake->read_header_calls++; + + *len_p = 0; + *type_p = GIT_OBJ_BLOB; + + return fake->error_code; +} + +static int fake_backend__read_prefix( + git_oid *out_oid, void **buffer_p, size_t *len_p, git_otype *type_p, + git_odb_backend *backend, const git_oid *short_oid, size_t len) +{ + fake_backend *fake; + + GIT_UNUSED(out_oid); + GIT_UNUSED(buffer_p); + GIT_UNUSED(len_p); + GIT_UNUSED(type_p); + GIT_UNUSED(short_oid); + GIT_UNUSED(len); + + fake = (fake_backend *)backend; + + fake->read_prefix_calls++; + + *len_p = 0; + *buffer_p = NULL; + *type_p = GIT_OBJ_BLOB; + + return fake->error_code; +} + +static void fake_backend__free(git_odb_backend *_backend) +{ + fake_backend *backend; + + backend = (fake_backend *)_backend; + + git__free(backend); +} + +static int build_fake_backend( + git_odb_backend **out, + git_error_code error_code) +{ + fake_backend *backend; + + backend = git__calloc(1, sizeof(fake_backend)); + GITERR_CHECK_ALLOC(backend); + + backend->parent.version = GIT_ODB_BACKEND_VERSION; + + backend->parent.refresh = NULL; + backend->error_code = error_code; + + backend->parent.read = fake_backend__read; + backend->parent.read_prefix = fake_backend__read_prefix; + backend->parent.read_header = fake_backend__read_header; + backend->parent.exists = fake_backend__exists; + backend->parent.free = &fake_backend__free; + + *out = (git_odb_backend *)backend; + + return 0; +} + +static void setup_repository_and_backend(git_error_code error_code) +{ + git_odb *odb; + git_odb_backend *backend; + + _repo = cl_git_sandbox_init("testrepo.git"); + + cl_git_pass(build_fake_backend(&backend, error_code)); + + cl_git_pass(git_repository_odb__weakptr(&odb, _repo)); + cl_git_pass(git_odb_add_backend(odb, backend, 10)); + + _fake = (fake_backend *)backend; + + cl_git_pass(git_oid_fromstr(&_oid, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef")); +} + +void test_odb_backend_nonrefreshing__cleanup(void) +{ + cl_git_sandbox_cleanup(); +} + +void test_odb_backend_nonrefreshing__exists_is_invoked_once_on_failure(void) +{ + git_odb *odb; + + setup_repository_and_backend(GIT_ENOTFOUND); + + cl_git_pass(git_repository_odb__weakptr(&odb, _repo)); + cl_assert_equal_b(false, git_odb_exists(odb, &_oid)); + + cl_assert_equal_i(1, _fake->exists_calls); +} + +void test_odb_backend_nonrefreshing__read_is_invoked_once_on_failure(void) +{ + git_object *obj; + + setup_repository_and_backend(GIT_ENOTFOUND); + + cl_git_fail_with( + git_object_lookup(&obj, _repo, &_oid, GIT_OBJ_ANY), + GIT_ENOTFOUND); + + cl_assert_equal_i(1, _fake->read_calls); +} + +void test_odb_backend_nonrefreshing__readprefix_is_invoked_once_on_failure(void) +{ + git_object *obj; + + setup_repository_and_backend(GIT_ENOTFOUND); + + cl_git_fail_with( + git_object_lookup_prefix(&obj, _repo, &_oid, 7, GIT_OBJ_ANY), + GIT_ENOTFOUND); + + cl_assert_equal_i(1, _fake->read_prefix_calls); +} + +void test_odb_backend_nonrefreshing__readheader_is_invoked_once_on_failure(void) +{ + git_odb *odb; + size_t len; + git_otype type; + + setup_repository_and_backend(GIT_ENOTFOUND); + + cl_git_pass(git_repository_odb__weakptr(&odb, _repo)); + + cl_git_fail_with( + git_odb_read_header(&len, &type, odb, &_oid), + GIT_ENOTFOUND); + + cl_assert_equal_i(1, _fake->read_header_calls); +} + +void test_odb_backend_nonrefreshing__exists_is_invoked_once_on_success(void) +{ + git_odb *odb; + + setup_repository_and_backend(GIT_OK); + + cl_git_pass(git_repository_odb__weakptr(&odb, _repo)); + cl_assert_equal_b(true, git_odb_exists(odb, &_oid)); + + cl_assert_equal_i(1, _fake->exists_calls); +} + +void test_odb_backend_nonrefreshing__read_is_invoked_once_on_success(void) +{ + git_object *obj; + + setup_repository_and_backend(GIT_OK); + + cl_git_pass(git_object_lookup(&obj, _repo, &_oid, GIT_OBJ_ANY)); + + cl_assert_equal_i(1, _fake->read_calls); + + git_object_free(obj); +} + +void test_odb_backend_nonrefreshing__readprefix_is_invoked_once_on_success(void) +{ + git_object *obj; + + setup_repository_and_backend(GIT_OK); + + cl_git_pass(git_object_lookup_prefix(&obj, _repo, &_oid, 7, GIT_OBJ_ANY)); + + cl_assert_equal_i(1, _fake->read_prefix_calls); + + git_object_free(obj); +} + +void test_odb_backend_nonrefreshing__readheader_is_invoked_once_on_success(void) +{ + git_odb *odb; + size_t len; + git_otype type; + + setup_repository_and_backend(GIT_OK); + + cl_git_pass(git_repository_odb__weakptr(&odb, _repo)); + + cl_git_pass(git_odb_read_header(&len, &type, odb, &_oid)); + + cl_assert_equal_i(1, _fake->read_header_calls); +}