Merge pull request libgit2#6107 from joshtriplett/refresh-handling

Support checking for object existence without refresh

Author: ethomson

include/git2/odb.h

@@ -22,6 +22,17 @@
  */
 GIT_BEGIN_DECL
 
+/** Flags controlling the behavior of ODB lookup operations */
+typedef enum {
+	/**
+	 * Don't call `git_odb_refresh` if the lookup fails. Useful when doing
+	 * a batch of lookup operations for objects that may legitimately not
+	 * exist. When using this flag, you may wish to manually call
+	 * `git_odb_refresh` before processing a batch of objects.
+	 */
+	GIT_ODB_LOOKUP_NO_REFRESH = (1 << 0)
+} git_odb_lookup_flags_t;
+
 /**
  * Function type for callbacks from git_odb_foreach.
  */
@@ -155,6 +166,17 @@ GIT_EXTERN(int) git_odb_read_header(size_t *len_out, git_object_t *type_out, git
  */
 GIT_EXTERN(int) git_odb_exists(git_odb *db, const git_oid *id);
 
+/**
+ * Determine if the given object can be found in the object database, with
+ * extended options.
+ *
+ * @param db database to be searched for the given object.
+ * @param id the object to search for.
+ * @param flags flags affecting the lookup (see `git_odb_lookup_flags_t`)
+ * @return 1 if the object was found, 0 otherwise
+ */
+GIT_EXTERN(int) git_odb_exists_ext(git_odb *db, const git_oid *id, unsigned int flags);
+
 /**
  * Determine if an object can be found in the object database by an
  * abbreviated object ID.

include/git2/sys/odb_backend.h

@@ -69,11 +69,8 @@ struct git_odb_backend {
 	 * 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()`).
+	 * The odb layer will automatically call this when needed on failed
+	 * lookups (ie. `exists()`, `read()`, `read_header()`).
 	 */
 	int GIT_CALLBACK(refresh)(git_odb_backend *);
 

src/odb.c

@@ -882,6 +882,11 @@ int git_odb__freshen(git_odb *db, const git_oid *id)
 }
 
 int git_odb_exists(git_odb *db, const git_oid *id)
+{
+    return git_odb_exists_ext(db, id, 0);
+}
+
+int git_odb_exists_ext(git_odb *db, const git_oid *id, unsigned int flags)
 {
 	git_odb_object *object;
 
@@ -899,7 +904,7 @@ int git_odb_exists(git_odb *db, const git_oid *id)
 	if (odb_exists_1(db, id, false))
 		return 1;
 
-	if (!git_odb_refresh(db))
+	if (!(flags & GIT_ODB_LOOKUP_NO_REFRESH) && !git_odb_refresh(db))
 		return odb_exists_1(db, id, true);
 
 	/* Failed to refresh, hence not found */

tests/odb/backend/backend_helpers.c

@@ -123,6 +123,18 @@ static int fake_backend__read_prefix(
 	return 0;
 }
 
+static int fake_backend__refresh(git_odb_backend *backend)
+{
+	fake_backend *fake;
+
+	fake = (fake_backend *)backend;
+
+	fake->refresh_calls++;
+
+	return 0;
+}
+
+
 static void fake_backend__free(git_odb_backend *_backend)
 {
 	fake_backend *backend;
@@ -134,7 +146,8 @@ static void fake_backend__free(git_odb_backend *_backend)
 
 int build_fake_backend(
 	git_odb_backend **out,
-	const fake_object *objects)
+	const fake_object *objects,
+	bool support_refresh)
 {
 	fake_backend *backend;
 
@@ -143,12 +156,12 @@ int build_fake_backend(
 
 	backend->parent.version = GIT_ODB_BACKEND_VERSION;
 
-	backend->parent.refresh = NULL;
 	backend->objects = objects;
 
 	backend->parent.read = fake_backend__read;
 	backend->parent.read_prefix = fake_backend__read_prefix;
 	backend->parent.read_header = fake_backend__read_header;
+	backend->parent.refresh = support_refresh ? fake_backend__refresh : NULL;
 	backend->parent.exists = fake_backend__exists;
 	backend->parent.exists_prefix = fake_backend__exists_prefix;
 	backend->parent.free = &fake_backend__free;

tests/odb/backend/backend_helpers.h

@@ -13,10 +13,12 @@ typedef struct {
 	int read_calls;
 	int read_header_calls;
 	int read_prefix_calls;
+	int refresh_calls;
 
 	const fake_object *objects;
 } fake_backend;
 
 int build_fake_backend(
 	git_odb_backend **out,
-	const fake_object *objects);
+	const fake_object *objects,
+	bool support_refresh);

tests/odb/backend/multiple.c

@@ -29,10 +29,10 @@ void test_odb_backend_multiple__initialize(void)
 	_obj = NULL;
 	_repo = cl_git_sandbox_init("testrepo.git");
 
-	cl_git_pass(build_fake_backend(&backend, _objects_filled));
+	cl_git_pass(build_fake_backend(&backend, _objects_filled, false));
 	_fake_filled = (fake_backend *)backend;
 
-	cl_git_pass(build_fake_backend(&backend, _objects_empty));
+	cl_git_pass(build_fake_backend(&backend, _objects_empty, false));
 	_fake_empty = (fake_backend *)backend;
 }
 

tests/odb/backend/nonrefreshing.c

@@ -23,7 +23,7 @@ static void setup_repository_and_backend(void)
 
 	_repo = cl_git_sandbox_init("testrepo.git");
 
-	cl_git_pass(build_fake_backend(&backend, _objects));
+	cl_git_pass(build_fake_backend(&backend, _objects, false));
 
 	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
 	cl_git_pass(git_odb_add_backend(odb, backend, 10));

tests/odb/backend/refreshing.c

@@ -0,0 +1,176 @@
+#include "clar_libgit2.h"
+#include "repository.h"
+#include "backend_helpers.h"
+
+static git_repository *_repo;
+static fake_backend *_fake;
+
+#define NONEXISTING_HASH "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+#define EXISTING_HASH "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"
+
+static const fake_object _objects[] = {
+	{ EXISTING_HASH, "" },
+	{ NULL, NULL }
+};
+
+static git_oid _nonexisting_oid;
+static git_oid _existing_oid;
+
+static void setup_repository_and_backend(void)
+{
+	git_odb *odb = NULL;
+	git_odb_backend *backend = NULL;
+
+	_repo = cl_git_sandbox_init("testrepo.git");
+
+	cl_git_pass(build_fake_backend(&backend, _objects, true));
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+	cl_git_pass(git_odb_add_backend(odb, backend, 10));
+
+	_fake = (fake_backend *)backend;
+}
+
+void test_odb_backend_refreshing__initialize(void)
+{
+	git_oid_fromstr(&_nonexisting_oid, NONEXISTING_HASH);
+	git_oid_fromstr(&_existing_oid, EXISTING_HASH);
+	setup_repository_and_backend();
+}
+
+void test_odb_backend_refreshing__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_odb_backend_refreshing__exists_is_invoked_twice_on_failure(void)
+{
+	git_odb *odb;
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+	cl_assert_equal_b(false, git_odb_exists(odb, &_nonexisting_oid));
+
+	cl_assert_equal_i(2, _fake->exists_calls);
+	cl_assert_equal_i(1, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__read_is_invoked_twice_on_failure(void)
+{
+	git_object *obj;
+
+	cl_git_fail_with(
+		git_object_lookup(&obj, _repo, &_nonexisting_oid, GIT_OBJECT_ANY),
+		GIT_ENOTFOUND);
+
+	cl_assert_equal_i(2, _fake->read_calls);
+	cl_assert_equal_i(1, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__readprefix_is_invoked_twice_on_failure(void)
+{
+	git_object *obj;
+
+	cl_git_fail_with(
+		git_object_lookup_prefix(&obj, _repo, &_nonexisting_oid, 7, GIT_OBJECT_ANY),
+		GIT_ENOTFOUND);
+
+	cl_assert_equal_i(2, _fake->read_prefix_calls);
+	cl_assert_equal_i(1, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__readheader_is_invoked_twice_on_failure(void)
+{
+	git_odb *odb;
+	size_t len;
+	git_object_t type;
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+
+	cl_git_fail_with(
+		git_odb_read_header(&len, &type, odb, &_nonexisting_oid),
+		GIT_ENOTFOUND);
+
+	cl_assert_equal_i(2, _fake->read_header_calls);
+	cl_assert_equal_i(1, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__exists_is_invoked_once_on_success(void)
+{
+	git_odb *odb;
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+	cl_assert_equal_b(true, git_odb_exists(odb, &_existing_oid));
+
+	cl_assert_equal_i(1, _fake->exists_calls);
+	cl_assert_equal_i(0, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__read_is_invoked_once_on_success(void)
+{
+	git_object *obj;
+
+	cl_git_pass(git_object_lookup(&obj, _repo, &_existing_oid, GIT_OBJECT_ANY));
+
+	cl_assert_equal_i(1, _fake->read_calls);
+	cl_assert_equal_i(0, _fake->refresh_calls);
+
+	git_object_free(obj);
+}
+
+void test_odb_backend_refreshing__readprefix_is_invoked_once_on_success(void)
+{
+	git_object *obj;
+
+	cl_git_pass(git_object_lookup_prefix(&obj, _repo, &_existing_oid, 7, GIT_OBJECT_ANY));
+
+	cl_assert_equal_i(1, _fake->read_prefix_calls);
+	cl_assert_equal_i(0, _fake->refresh_calls);
+
+	git_object_free(obj);
+}
+
+void test_odb_backend_refreshing__readheader_is_invoked_once_on_success(void)
+{
+	git_odb *odb;
+	size_t len;
+	git_object_t type;
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+
+	cl_git_pass(git_odb_read_header(&len, &type, odb, &_existing_oid));
+
+	cl_assert_equal_i(1, _fake->read_header_calls);
+	cl_assert_equal_i(0, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__read_is_invoked_twice_when_revparsing_a_full_oid(void)
+{
+	git_object *obj;
+
+	cl_git_fail_with(
+		git_revparse_single(&obj, _repo, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
+		GIT_ENOTFOUND);
+
+	cl_assert_equal_i(2, _fake->read_calls);
+	cl_assert_equal_i(1, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__refresh_is_invoked(void)
+{
+	git_odb *odb;
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+	cl_assert_equal_i(0, git_odb_refresh(odb));
+
+	cl_assert_equal_i(1, _fake->refresh_calls);
+}
+
+void test_odb_backend_refreshing__refresh_suppressed_with_no_refresh(void)
+{
+	git_odb *odb;
+
+	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
+	cl_assert_equal_b(false, git_odb_exists_ext(odb, &_nonexisting_oid, GIT_ODB_LOOKUP_NO_REFRESH));
+
+	cl_assert_equal_i(0, _fake->refresh_calls);
+}

tests/odb/backend/simple.c

@@ -13,7 +13,7 @@ static void setup_backend(const fake_object *objs)
 {
 	git_odb_backend *backend;
 
-	cl_git_pass(build_fake_backend(&backend, objs));
+	cl_git_pass(build_fake_backend(&backend, objs, false));
 
 	cl_git_pass(git_repository_odb__weakptr(&_odb, _repo));
 	cl_git_pass(git_odb_add_backend(_odb, backend, 10));