modules/kvs: Remove cache_lookup_and_get_treeobj

The function was more widely used before, but is now only used
in two locations.  Remove function and replace with calls to other
functions.

Remove unit tests for cache_lookup_and_get_treeobj().

Adjust callers appropriately.

src/modules/kvs/cache.c

@@ -294,14 +294,6 @@ struct cache_entry *cache_lookup (struct cache *cache, const char *ref,
     return hp;
 }
 
-json_t *cache_lookup_and_get_treeobj (struct cache *cache,
-                                      const char *ref,
-                                      int current_epoch)
-{
-    struct cache_entry *hp = cache_lookup (cache, ref, current_epoch);
-    return cache_entry_get_valid (hp) ? cache_entry_get_treeobj (hp) : NULL;
-}
-
 void cache_insert (struct cache *cache, const char *ref, struct cache_entry *hp)
 {
     int rc = zhash_insert (cache->zh, ref, hp);

src/modules/kvs/cache.h

@@ -105,15 +105,6 @@ void cache_destroy (struct cache *cache);
 struct cache_entry *cache_lookup (struct cache *cache,
                                   const char *ref, int current_epoch);
 
-/* Look up a cache entry and get treeobj of cache entry only if entry
- * contains a valid treeobj.  This is a convenience function that is
- * effectively successful if calls to cache_lookup() and
- * cache_entry_get_treeobj() are both successful.
- */
-json_t *cache_lookup_and_get_treeobj (struct cache *cache,
-                                      const char *ref,
-                                      int current_epoch);
-
 /* Insert an entry in the cache by blobref 'ref'.
  * Ownership of the cache entry is transferred to the cache.
  */

src/modules/kvs/commit.c

@@ -557,6 +557,7 @@ static int commit_link_dirent (commit_t *c, int current_epoch,
         } else if (treeobj_is_dir (dir_entry)) {
             subdir = dir_entry;
         } else if (treeobj_is_dirref (dir_entry)) {
+            struct cache_entry *hp;
             const char *ref;
             int refcount;
 
@@ -577,13 +578,17 @@ static int commit_link_dirent (commit_t *c, int current_epoch,
                 goto done;
             }
 
-            if (!(subdir = cache_lookup_and_get_treeobj (c->cm->cache,
-                                                         ref,
-                                                         current_epoch))) {
+            if (!(hp = cache_lookup (c->cm->cache, ref, current_epoch))
+                || !cache_entry_get_valid (hp)) {
                 *missing_ref = ref;
                 goto success; /* stall */
             }
 
+            if (!(subdir = cache_entry_get_treeobj (hp))) {
+                saved_errno = ENOTRECOVERABLE;
+                goto done;
+            }
+
             /* do not corrupt store by modifying orig. */
             if (!(subdir = treeobj_copy (subdir))) {
                 saved_errno = errno;
@@ -693,6 +698,7 @@ commit_process_t commit_process (commit_t *c,
         {
             /* Make a copy of the root directory.
              */
+            struct cache_entry *hp;
             json_t *rootdir;
 
             /* Caller didn't call commit_iter_missing_refs() */
@@ -701,9 +707,11 @@ commit_process_t commit_process (commit_t *c,
 
             c->state = COMMIT_STATE_LOAD_ROOT;
 
-            if (!(rootdir = cache_lookup_and_get_treeobj (c->cm->cache,
-                                                          rootdir_ref,
-                                                          current_epoch))) {
+            if (!(hp = cache_lookup (c->cm->cache,
+                                     rootdir_ref,
+                                     current_epoch))
+                || !cache_entry_get_valid (hp)) {
+
                 if (zlist_push (c->missing_refs_list,
                                 (void *)rootdir_ref) < 0) {
                     c->errnum = ENOMEM;
@@ -712,6 +720,11 @@ commit_process_t commit_process (commit_t *c,
                 goto stall_load;
             }
 
+            if (!(rootdir = cache_entry_get_treeobj (hp))) {
+                c->errnum = ENOTRECOVERABLE;
+                return COMMIT_PROCESS_ERROR;
+            }
+
             if (!(c->rootcpy = treeobj_copy (rootdir))) {
                 c->errnum = errno;
                 return COMMIT_PROCESS_ERROR;

src/modules/kvs/test/cache.c

@@ -600,12 +600,8 @@ void cache_expiration_tests (void)
         "cache contains 1 entry after insert");
     ok (cache_lookup (cache, "yyy1", 0) == NULL,
         "cache_lookup of wrong hash fails");
-    ok (cache_lookup_and_get_treeobj (cache, "yyy1", 0) == NULL,
-        "cache_lookup_and_get_treeobj of wrong hash fails");
     ok ((e2 = cache_lookup (cache, "xxx1", 42)) != NULL,
         "cache_lookup of correct hash works (last use=42)");
-    ok (cache_lookup_and_get_treeobj (cache, "xxx1", 0) == NULL,
-        "cache_lookup_and_get_treeobj of correct hash, but non valid entry fails");
     ok (cache_entry_get_treeobj (e2) == NULL,
         "no treeobj object found");
     ok (cache_count_entries (cache) == 1,
@@ -642,10 +638,6 @@ void cache_expiration_tests (void)
     ok ((otmp = cache_entry_get_treeobj (e4)) != NULL,
         "cache_entry_get_treeobj found entry");
     otest = treeobj_create_val ("foo", 3);
-    ok (json_equal (otmp, otest) == 1,
-        "expected treeobj object found");
-    ok ((otmp = cache_lookup_and_get_treeobj (cache, "xxx2", 0)) != NULL,
-        "cache_lookup_and_get_treeobj of correct hash and valid entry works");
     ok (json_equal (otmp, otest) == 1,
         "expected treeobj object found");
     json_decref (otest);