commercetools / commercetools-sync-java

Java library for importing and syncing (taking care of changes) data into one or more commercetools projects from external data files or from another commercetools project.
https://commercetools.github.io/commercetools-sync-java
Apache License 2.0
32 stars 37 forks source link

Fix NullpointerException for products having a category but no order-hint #1126

Closed dfrommi closed 9 months ago

dfrommi commented 9 months ago

Summary

The Product-Reference-Resolution is not covering the case where a product is assigned to a category, but has no order-hint set.

Description

Class CategoryOrderHints from the Commercetools-SDK has a values() method that is backed by a Map. The Map is only initialised when a order-hint is set, otherwise values() returns null. So in case of a product having a category, but no order-hint, the categoryOrderHints of the product is not null, but its values-methods returns null.

This case was not covered, leading to a NullpointerException like

java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because the return value of "com.commercetools.api.models.product.CategoryOrderHints.values()" is null
    at com.commercetools.sync.products.utils.ProductReferenceResolutionUtils.lambda$mapToCategoryReferencePair$7(ProductReferenceResolutionUtils.java:288)
    at java.base/java.util.ArrayList.forEach(Unknown Source)
    at com.commercetools.sync.products.utils.ProductReferenceResolutionUtils.mapToCategoryReferencePair(ProductReferenceResolutionUtils.java:280)
    at com.commercetools.sync.products.utils.ProductReferenceResolutionUtils.lambda$mapToProductDrafts$3(ProductReferenceResolutionUtils.java:135)
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(Unknown Source)
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
    at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(Unknown Source)
...

Relevant Issues

Todo

Hints for Review

Example-extract from ImpEx of a real product-projection:

  "categories": [
    {
      "typeId": "category",
      "id": "0194530c-2128-4da6-982b-7c2c876591f1"
    }
  ],
  "categoryOrderHints": {},
salander85 commented 9 months ago

@dfrommi Thanks for your contribution on this repo by fixing this issue. Please update your master branch and update the PR. The code checks need to be run successfully before merging.

dfrommi commented 9 months ago

Thanks @salander85 I've updated the branch, but also realised that I missed one location in my initial commit. Could you please have another look?

dfrommi commented 9 months ago

Do I need to do anything to trigger the code checks?

salander85 commented 9 months ago

Do I need to do anything to trigger the code checks?

I've created a PR in your fork to update your master. Please merge your branch into your updated master and update this PR to merge into commercetools master. I hope this will fix the code checks.

dfrommi commented 9 months ago

Thanks. I've now updated my fork's master to the commit of this master and rebased my branch on it. Hope that helps

dfrommi commented 9 months ago

Hm, doesn't really look like it. How about you create a new PR with this content? @salander85

salander85 commented 9 months ago

Hm, doesn't really look like it. How about you create a new PR with this content? @salander85

Okay, therefore i will close this PR. @dfrommi Please merge your local branch into your forked master-branch. Then it should be possible to create a PR from your fork to origin master. thank you.