Skip to content

Commit

Permalink
Merge pull request #23 from openfoodfacts:test-failure
Browse files Browse the repository at this point in the history 
Use a different tag to test loaded tags is updated
  • Loading branch information
@john-gom
john-gom authored Nov 13, 2023
2 parents feae0f2 + be6aa26 commit 78c4736
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/domain/entities/product-tags.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export const MAPPED_TAGS = {
entry_dates_tags: ProductEntryDatesTag,
last_edit_dates_tags: ProductLastEditDatesTag,
last_check_dates_tags: ProductLastCheckDatesTag,
// Note do not use the teams_tags in test data as it is deleted in on eof the tests
teams_tags: ProductTeamsTag,
// Added later
_keywords: ProductKeywordsTag,
Expand Down
54 changes: 37 additions & 17 deletions src/domain/services/import.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ jest.mock('mongodb', () => {
collection: () => ({
find: () => ({
next: async () => {
return index++ <= mockedProducts.length ? mockedProducts[index - 1] : null;
return index++ <= mockedProducts.length
? mockedProducts[index - 1]
: null;
},
close: jest.fn(),
}),
Expand Down Expand Up @@ -61,7 +63,7 @@ describe('importFromMongo', () => {
// app.useLogger(new Logger());

const importService = app.get(ImportService);
importService.deleteAllProducts = jest.fn();
const deleteMock = (importService.deleteOtherProducts = jest.fn());

// GIVEN: Two existing products, one of which is in Mongo plus one new one in Mongo
const em = app.get(EntityManager);
Expand All @@ -77,16 +79,28 @@ describe('importFromMongo', () => {
product: productUnchanged,
value: 'unchanged_ingredient',
});
// Delete a tag to prove it is re-created
await em.nativeDelete(LoadedTag, { id: 'teams_tags' });
await em.flush();

// WHEN:Doing a full import from MongoDB
mockMongoDB(products);
await importService.importFromMongo();

// THEN: New product is addeded, updated product is updated and other product is unchanged
expect(importService.deleteAllProducts).not.toHaveBeenCalled();
expect(deleteMock).toHaveBeenCalledTimes(1);
let updateId = deleteMock.mock.calls[0][0];
// Re-format updateId the way Postgres provides it
updateId = `${updateId.substring(0, 8)}-${updateId.substring(
8,
12,
)}-${updateId.substring(12, 16)}-${updateId.substring(
16,
20,
)}-${updateId.substring(20)}`.toLowerCase();
const productNew = await em.findOne(Product, { code: productIdNew });
expect(productNew).toBeTruthy();
expect(productNew.lastUpdateId).toBe(updateId);
const ingredientsNew = await em.find(ProductIngredientsTag, {
product: productNew,
});
Expand All @@ -104,10 +118,12 @@ describe('importFromMongo', () => {
ingredientsExisting.find((i) => i.value === 'new_ingredient'),
).toBeTruthy();

// We have mocked the delete of other products so just check the other product
// does not have the same update id as those imported
const foundOldProduct = await em.findOne(Product, {
code: productIdUnchanged,
});
expect(foundOldProduct).toBeFalsy();
expect(foundOldProduct.lastUpdateId).not.toBe(updateId);

const loadedTags = await app.get(TagService).getLoadedTags();
expect(loadedTags).toHaveLength(Object.keys(MAPPED_TAGS).length);
Expand All @@ -116,9 +132,9 @@ describe('importFromMongo', () => {

it('incremental import should not update loaded tags', async () => {
await createTestingModule([DomainModule], async (app) => {
// GIVEN: No loaded ingredients tag
// GIVEN: No loaded teams tag
const em = app.get(EntityManager);
await em.nativeDelete(LoadedTag, { id: 'ingredients_tags' });
await em.nativeDelete(LoadedTag, { id: 'teams_tags' });
await em.flush();
const importService = app.get(ImportService);

Expand All @@ -128,28 +144,32 @@ describe('importFromMongo', () => {

// THEN: Loaded tags is not updated
const loadedTags = await app.get(TagService).getLoadedTags();
expect(loadedTags).not.toContain('ingredients_tags');
expect(loadedTags).not.toContain('teams_tags');
});
});

it('should cope with nul charactes', async () => {
await createTestingModule([DomainModule], async (app) => {
// WHEN: Impoting data containing nul characters
mockMongoDB([{
// This one will be new
code: productIdNew,
last_modified_t: 1692032161,
ingredients_tags: ["test \u0000 test2 \u0000 end"],
}]);
mockMongoDB([
{
// This one will be new
code: productIdNew,
last_modified_t: 1692032161,
ingredients_tags: ['test \u0000 test2 \u0000 end'],
},
]);
await app.get(ImportService).importFromMongo();

// THEN: Product should be loaded with nuls stripped
const ingredientsNew = await app.get(EntityManager).find(ProductIngredientsTag, {
product: { code: productIdNew},
});
const ingredientsNew = await app
.get(EntityManager)
.find(ProductIngredientsTag, {
product: { code: productIdNew },
});

expect(ingredientsNew).toHaveLength(1);
expect(ingredientsNew[0].value).toBe("test test2 end");
expect(ingredientsNew[0].value).toBe('test test2 end');
});
});
});
28 changes: 17 additions & 11 deletions src/domain/services/import.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,7 @@ export class ImportService {

// If doing a full import delete all products that weren't updated
if (!from) {
const deleted = await this.em.nativeDelete(Product, {
$or: [{ lastUpdateId: { $ne: updateId } }, { lastUpdateId: null }],
});
this.logger.log(`${deleted} Products deleted`);
await this.deleteOtherProducts(updateId);
}
this.logger.log('Finished');
}
Expand All @@ -119,7 +116,9 @@ export class ImportService {
// Strip out any nul characters
for (const [index, value] of tagData.entries()) {
if (value.includes('\u0000')) {
this.logger.warn(`Product: ${data.code}. Nuls stripped from ${key} value: ${value}`);
this.logger.warn(
`Product: ${data.code}. Nuls stripped from ${key} value: ${value}`,
);
tagData[index] = value.replace(this.nulRegex, '');
}
}
Expand All @@ -134,7 +133,9 @@ export class ImportService {
product.obsolete = obsolete;
const lastModified = new Date(data.last_modified_t * 1000);
if (isNaN(+lastModified)) {
this.logger.warn(`Product: ${data.code}. Invalid last_modified_t: ${data.last_modified_t}.`);
this.logger.warn(
`Product: ${data.code}. Invalid last_modified_t: ${data.last_modified_t}.`,
);
} else {
product.lastModified = lastModified;
}
Expand Down Expand Up @@ -203,8 +204,11 @@ export class ImportService {
}
}

async deleteAllProducts() {
await this.em.execute('truncate table product cascade');
async deleteOtherProducts(updateId: string) {
const deleted = await this.em.nativeDelete(Product, {
$or: [{ lastUpdateId: { $ne: updateId } }, { lastUpdateId: null }],
});
this.logger.log(`${deleted} Products deleted`);
}

/**
Expand All @@ -218,8 +222,6 @@ export class ImportService {
input: fs.createReadStream('data/openfoodfacts-products.jsonl'),
});

if (!from) await this.deleteAllProducts();
//await this.cacheTags();
let i = 0;
let skip = 0;
for await (const line of rl) {
Expand All @@ -242,7 +244,7 @@ export class ImportService {

const data = JSON.parse(line.replace(/\\u0000/g, ''));
i++;
await this.fixupProduct(!!from, updateId, data);
await this.fixupProduct(true, updateId, data);
if (!(i % this.importBatchSize)) {
await this.em.flush();
this.em.clear();
Expand All @@ -257,5 +259,9 @@ export class ImportService {
await this.em.flush();
this.logger.log(`${i} Products imported`);
await this.updateTags(!!from, updateId);
if (!from) {
await this.deleteOtherProducts(updateId);
}
this.logger.log('Finished');
}
}

0 comments on commit 78c4736

Please sign in to comment.