- Keep track of what url requests returning "not found" so we don't r…

…epeat them.

include/sfc/sfc_cache.h

@@ -17,6 +17,7 @@ typedef struct _sfc_cache
 	struct _sfc_string_set*			full_sets;
 	struct _sfc_card_set*			card_set;	
 	struct _sfc_card_map_uint32*	cardmarket_id_map;
+	struct _sfc_string_set*			bad_urls;
 
 	void*							http_context;
 

sfc/sfc_cache.c

@@ -39,6 +39,7 @@ sfc_cache_create(
 
 	cache->http_request_rate_limiter = sfc_leaky_bucket_create(app, max_http_requests_per_second, max_http_requests_per_second);
 	cache->full_sets = sfc_string_set_create(app);
+	cache->bad_urls = sfc_string_set_create(app);
 	cache->card_set = sfc_card_set_create(app);
 	cache->cardmarket_id_map = sfc_card_map_uint32_create(app, 1);
 
@@ -58,6 +59,7 @@ sfc_cache_destroy(
 
 	sfc_leaky_bucket_destroy(cache->http_request_rate_limiter);
 	sfc_string_set_destroy(cache->full_sets);
+	sfc_string_set_destroy(cache->bad_urls);
 	sfc_card_set_destroy(cache->card_set);
 	sfc_card_map_uint32_destroy(cache->cardmarket_id_map);
 
@@ -248,7 +250,7 @@ sfc_cache_save(
 		if (f == NULL)
 			return SFC_RESULT_FILE_OPEN_ERROR;
 
-		/* First part: header and list of full sets */
+		/* First part: header, list of full sets, and list of bad urls */
 		{
 			sfc_buffer buffer;
 			sfc_buffer_init(&buffer, cache->app);
@@ -257,9 +259,10 @@ sfc_cache_save(
 				sfc_serializer serializer;
 				sfc_serializer_init(&serializer, &buffer);
 
-				sfc_serializer_write_uint8(&serializer, 2); /* Version */
+				sfc_serializer_write_uint8(&serializer, 3); /* Version */
 
 				sfc_string_set_serialize(cache->full_sets, &serializer);
+				sfc_string_set_serialize(cache->bad_urls, &serializer);
 			}
 
 			result = sfc_file_write_buffer(f, &buffer);
@@ -322,15 +325,18 @@ sfc_cache_load(
 
 				result = sfc_deserializer_read_uint8(&deserializer, &format_version);
 
-				if(format_version != 2)
+				if (format_version != 2 && format_version != 3)
 				{
-					/* We can't read this old version, saved cached is invalidated. */
+					/* We can't read this old (or new?) version, saved cached is invalidated. */
 					/* Behave like if the file didn't exist. */
 					result = SFC_RESULT_FILE_OPEN_ERROR;
 				}
 
 				if(result == SFC_RESULT_OK)
 					result = sfc_string_set_deserialize(cache->full_sets, &deserializer);
+
+				if(result == SFC_RESULT_OK && format_version >= 3)
+					result = sfc_string_set_deserialize(cache->bad_urls, &deserializer);
 			}
 
 			sfc_buffer_uninit(&buffer);

sfc/sfc_query.c

@@ -14,6 +14,35 @@
 #include "sfc_leaky_bucket.h"
 #include "sfc_string_set.h"
 
+static void
+sfc_query_handle_set_card_not_found(
+	sfc_query*			query)
+{
+	if (query->next_collector_number_version > 0)
+	{
+		if (query->next_collector_number_version == 1)
+		{
+			/* Must have reached the end since neither "x" nor "xa" exists. */
+			query->result = SFC_RESULT_OK;
+			query->state = SFC_QUERY_STATE_COMPLETED;
+
+			/* We're now sure to have the full set, mark it as fully cached */
+			sfc_string_set_add(query->cache->full_sets, query->u.set);
+		}
+		else
+		{
+			query->next_collector_number_version = 0;
+			query->next_collector_number++;
+			query->state = SFC_QUERY_STATE_GET_NEXT_IN_SET;
+		}
+	}
+	else
+	{
+		query->next_collector_number_version = 1;
+		query->state = SFC_QUERY_STATE_GET_NEXT_IN_SET;
+	}
+}
+
 /* Update state: SFC_QUERY_STATE_INIT */
 static void
 sfc_query_update_init(
@@ -213,6 +242,21 @@ static void
 sfc_query_update_http_get(
 	sfc_query*			query)
 {
+	if(sfc_string_set_has(query->cache->bad_urls, query->http_get_url))
+	{
+		if(query->type == SFC_QUERY_TYPE_SET)
+		{
+			sfc_query_handle_set_card_not_found(query);
+		}
+		else
+		{
+			query->state = SFC_QUERY_STATE_COMPLETED;
+			query->result = SFC_RESULT_NOT_FOUND;
+		}
+
+		return;
+	}
+
 	if (!sfc_leaky_bucket_acquire_token(query->cache->http_request_rate_limiter))
 		return;
 
@@ -286,29 +330,9 @@ sfc_query_update_parse_result(
 		}
 		else if (result == SFC_RESULT_NOT_FOUND)
 		{
-			if (query->next_collector_number_version > 0)
-			{
-				if (query->next_collector_number_version == 1)
-				{
-					/* Must have reached the end since neither "x" nor "xa" exists. */
-					query->result = SFC_RESULT_OK;
-					query->state = SFC_QUERY_STATE_COMPLETED;
+			sfc_string_set_add(query->cache->bad_urls, query->http_get_url);
 
-					/* We're now sure to have the full set, mark it as fully cached */
-					sfc_string_set_add(query->cache->full_sets, query->u.set);
-				}
-				else
-				{
-					query->next_collector_number_version = 0;
-					query->next_collector_number++;
-					query->state = SFC_QUERY_STATE_GET_NEXT_IN_SET;
-				}
-			}
-			else
-			{
-				query->next_collector_number_version = 1;
-				query->state = SFC_QUERY_STATE_GET_NEXT_IN_SET;
-			}
+			sfc_query_handle_set_card_not_found(query);
 		}
 		else
 		{
@@ -365,6 +389,9 @@ sfc_query_update_parse_result(
 		}
 		else
 		{
+			if(result == SFC_RESULT_NOT_FOUND)
+				sfc_string_set_add(query->cache->bad_urls, query->http_get_url);
+
 			query->result = result;
 			query->state = SFC_QUERY_STATE_COMPLETED;
 		}

sfc/sfc_string_set.c

@@ -29,15 +29,15 @@ sfc_string_set_destroy(
 	string_set->app->free(string_set->app->user_data, string_set);
 }
 
-sfc_result			
+void			
 sfc_string_set_add(
 	sfc_string_set*		string_set,
 	const char*			string)
 {
 	size_t len;
 
 	if(sfc_string_set_has(string_set, string))
-		return SFC_RESULT_OK;
+		return;
 
 	len = strlen(string);
 
@@ -50,8 +50,6 @@ sfc_string_set_add(
 	strcpy(string_set->strings[string_set->count], string);
 
 	string_set->count++;
-
-	return SFC_RESULT_OK;
 }
 
 int					

sfc/sfc_string_set.h

@@ -22,7 +22,7 @@ sfc_string_set*		sfc_string_set_create(
 void				sfc_string_set_destroy(
 						sfc_string_set*				string_set);
 
-sfc_result			sfc_string_set_add(
+void				sfc_string_set_add(
 						sfc_string_set*				string_set,
 						const char*					string);
 

test/test.c

@@ -67,7 +67,7 @@ test_assert(
 {
 	if (!result)
 	{
-		printf("FAILED: %s (%s:%d)\n", message, file, line);
+		fprintf(stderr, "FAILED: %s (%s:%d)\n", message, file, line);
 
 		exit(1);
 	}
@@ -96,6 +96,8 @@ typedef struct _test_user_data
 
 	int							auto_advance_timer;
 	uint64_t					test_timer;
+
+	size_t						total_http_request_count;
 } test_user_data;
 
 void
@@ -288,12 +290,22 @@ test_http_get(
 	req->context = context;
 
 	context->request_count++;
+
+	test_user_data* user_data = (test_user_data*)context->app->user_data;
+
+	user_data->total_http_request_count++;
 
 	if(strcmp(url, "https://api.scryfall.com/cards/cardmarket/5013") == 0 ||
 		strcmp(url, "https://api.scryfall.com/cards/2ed/9") == 0)
 	{		
 		req->result = test_strdup_no_null_term(context->app, g_test_json_cardmarket_id_5013, &req->size);
 	}
+	else if (strcmp(url, "https://api.scryfall.com/cards/cardmarket/123456") == 0)
+	{
+		req->result = test_strdup_no_null_term(context->app,
+			"{\"object\":\"error\", \"code\":\"not_found\"}",
+			&req->size);
+	}
 	else if(strcmp(url, "https://api.scryfall.com/cards/test/1") == 0)
 	{
 		req->result = test_strdup_no_null_term(context->app,
@@ -469,6 +481,71 @@ test_cache()
 	sfc_cache* cache = sfc_cache_create(&app, 9);
 	TEST_ASSERT(cache != NULL);
 
+	/* Query a cardmarket id that doesn't exist */
+	{
+		sfc_query* query = sfc_cache_query_cardmarket_id(cache, 123456);
+
+		size_t request_count_0 = test_data.total_http_request_count;
+
+		/* Update query until completed */
+		{
+			TEST_ASSERT(query != NULL);
+			TEST_ASSERT(sfc_query_wait(query, TEST_QUERY_TIMEOUT) == SFC_RESULT_OK);
+		}
+
+		/* Verify that we got the expected result */
+		{
+			TEST_ASSERT(query->result = SFC_RESULT_NOT_FOUND);
+			TEST_ASSERT(query->results->count == 0);
+		}
+
+		size_t request_count_1 = test_data.total_http_request_count;
+
+		/* Should have triggered 1 http request */
+		TEST_ASSERT(request_count_1 - request_count_0 == 1);
+
+		/* Delete query */
+		{
+			sfc_query_delete(query);
+			sfc_cache_update(cache);
+			TEST_ASSERT(cache->first_query == NULL);
+			TEST_ASSERT(cache->last_query == NULL);
+		}
+	}
+
+	/* Query a cardmarket id that doesn't exist again. This time it shouldn't trigger
+	   any http requests because the url will be in the "bad url" list */
+	{
+		sfc_query* query = sfc_cache_query_cardmarket_id(cache, 123456);
+
+		size_t request_count_0 = test_data.total_http_request_count;
+
+		/* Update query until completed */
+		{
+			TEST_ASSERT(query != NULL);
+			TEST_ASSERT(sfc_query_wait(query, TEST_QUERY_TIMEOUT) == SFC_RESULT_OK);
+		}
+
+		/* Verify that we got the expected result */
+		{
+			TEST_ASSERT(query->result = SFC_RESULT_NOT_FOUND);
+			TEST_ASSERT(query->results->count == 0);
+		}
+
+		size_t request_count_1 = test_data.total_http_request_count;
+
+		/* No http requests expected */
+		TEST_ASSERT(request_count_1 == request_count_0);
+
+		/* Delete query */
+		{
+			sfc_query_delete(query);
+			sfc_cache_update(cache);
+			TEST_ASSERT(cache->first_query == NULL);
+			TEST_ASSERT(cache->last_query == NULL);
+		}
+	}
+
 	/* Query cardmarket id 5013 on an empty cache */
 	{
 		sfc_query* query = sfc_cache_query_cardmarket_id(cache, 5013);