Commit b1a6c316a6070fac4ab1ec5792979838f7145c39

nulltoken 2013-08-30T17:36:00

odb: Move the auto refresh logic to the pack backend Previously, `git_object_read()`, `git_object_read_prefix()` and `git_object_exists()` were implementing an auto refresh logic. When the expected object couldn't be found in any backend, a call to `git_odb_refresh()` was triggered and the lookup was once again performed against all backends. This commit removes this auto-refresh logic from the odb layer and pushes it down into the pack-backend (as it's the only one currently exposing a `refresh()` endpoint).

diff --git a/include/git2/sys/odb_backend.h b/include/git2/sys/odb_backend.h
index 31ffe1c..4365906 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 9785c74..e47715f 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,25 +616,14 @@ 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;
 
-		if (b->exists != NULL && (!refreshed || b->refresh))
+		if (b->exists != NULL)
 			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,27 +697,18 @@ 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) {
 		backend_internal *internal = git_vector_get(&db->backends, i);
 		git_odb_backend *b = internal->backend;
 
-		if (b->read != NULL && (!refreshed || b->refresh)) {
+		if (b->read != NULL) {
 			++reads;
 			error = b->read(&raw.data, &raw.len, &raw.type, b, id);
 		}
 	}
 
-	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,12 +747,11 @@ 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;
 
-		if (b->read_prefix != NULL && (!refreshed || b->refresh)) {
+		if (b->read_prefix != NULL) {
 			git_oid full_oid;
 			error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, short_id, len);
 			if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH)
@@ -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 d7abd10..d24b4aa 100644
--- a/src/odb_pack.c
+++ b/src/odb_pack.c
@@ -342,7 +342,7 @@ static int pack_backend__refresh(git_odb_backend *_backend)
 	return 0;
 }
 
-static int pack_backend__read_header(
+static int pack_backend__read_header_internal(
 	size_t *len_p, git_otype *type_p,
 	struct git_odb_backend *backend, const git_oid *oid)
 {
@@ -357,7 +357,24 @@ static int pack_backend__read_header(
 	return git_packfile_resolve_header(len_p, type_p, e.p, e.offset);
 }
 
-static int pack_backend__read(
+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)
 {
@@ -376,7 +393,24 @@ static int pack_backend__read(
 	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,
@@ -413,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;
 }