Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jackson-dataformat-csv:2.8.11/2.12.2]CsvMapper unrecognised the @JsonProperty #268

Open
zhaoxi2000 opened this issue Apr 29, 2021 · 10 comments
Labels
csv need-test-case Further progress requires inclusion of problem reproduction, ideally test case

Comments

@zhaoxi2000
Copy link

zhaoxi2000 commented Apr 29, 2021

the result is
image

AppsFlyerRawDataBean.java.txt
appsflyer-raw-data.csv

        final CsvMapper mapper = new CsvMapper();
        final CsvSchema schema = mapper.schemaFor(AppsFlyerRawDataBean.class);
        final ObjectReader objectReader = mapper.readerFor(AppsFlyerRawDataBean.class).with(schema);
        final Collection<AppsFlyerRawData> result = new ArrayList<>(20);

        try (final Reader reader = new FileReader(csv)) {
            final MappingIterator<AppsFlyerRawDataBean> records = objectReader.readValues(reader);
            while (records.hasNextValue()) {
                final AppsFlyerRawDataBean one = records.nextValue();
                if (isBlank(one)) {
                    continue;
                }
                result.add(one);
            }
        }
@cowtowncoder
Copy link
Member

First things, first: PLEASE DO NOT USE

@JsonIgnoreProperties(ignoreUnknown = true)

on tests. This is VERY common way to hide simple problems that you could fix if you saw them.
Only use that in production once other problems are resolved (if at all).

Second: if input contains header line (which I think is the case here), your schema must indicate this:

 schema = schema.withUseHeader(true);

You probably do not need create schema from POJO class either, as long as names from header line match expected property names (either from getter name, or via annotation).

With these changes you may be able to get better results and see if there are other problems.

@zhaoxi2000
Copy link
Author

@cowtowncoder
Do not use @JsonIgnoreProperties(ignoreUnknown = true) , I think it is not correct way because csv has 90 columns and programmer just need extracts 30 columns. CSV comes from the collaboration company.

Second: schema = schema.withUseHeader(true); I had a try a few days ago with the same result.

POJO property names does not match the CSV header. You can open the two files: appsflyer-raw-data.csv

@cowtowncoder cowtowncoder added the need-test-case Further progress requires inclusion of problem reproduction, ideally test case label May 9, 2021
@cowtowncoder
Copy link
Member

To me header line names do seem to match annotated names?

  • Campaign on header row matches @JosonProperty("Campaign") of private String campaignName
  • Campaign ID similarly matches campaignId property

At this point I would need either:

  • Specific exception message of what fails? Which property/column is mismatching OR
  • Full unit test to reproduce the issue

@zhaoxi2000
Copy link
Author

No any exception on fails.
Do you need my full test-case?

@cowtowncoder
Copy link
Member

A test case that shows the problem with assertions would help: the original picture simply showed a POJO with values, some of which were nulls. That does not really tell me much about what the problem is: what @JsonProperty seems to be ignored? (which POJO property should have gotten value from CSV but didn't?).

@sbszcz
Copy link

sbszcz commented Jun 14, 2021

Hello @cowtowncoder ,

I recently came across that issue because I faced the same problem within one of my projects.
I created a repo containing a (hopefully meaningful) test case.

https://github.com/sbszcz/jackson-csv-java-example

Mapping the content of the columns does not work. I have no idea why. The content seems to be mapped to arbitrary pojo properties.

Could you please have a look at this?

Thank you,
Sebastian

@cowtowncoder
Copy link
Member

Looking at the example @sbszcz (thank you!) -- my first instinct is that I don't understand why there is no fail for non-annotated case since there is no match from header names to actual names.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jul 9, 2021

@sbszcz A few issues with the code, solving of which will make things work.

First: you should not usually (need to) combine 2 types of CsvSchemas (one generated from POJOs, using specific ordering; another read from header) -- in this case I think only latter makes sense.
Sometimes there are benefits from combining this way; but if so, you almost certainly want to use this line too:

csvSchema = csvSchema.withColumnReordering(true);

which ensures that the information is combined: otherwise explicitly specified schema (from POJO, and ordering it defines) is used.

So, instead of starting with mapper.schemaFor(Type.class), start with CsvSchema.emptySchema().
Second, since there are entries in input that do not map to POJO (at least in the test), disable DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES.
This makes test with annotations pass.

The problem without using annotations is that there is no mapping between names from CSV file header (in German) and property names in PlainTransaction, so that cannot pass quite as specified.
It would be possible to programmatically build CsvSchema, and then force skipping of the first (data) row:

schema = schema.withSkipFirstDataRow(true);

Or, even specify the expected ordering for POJO Fields with @JsonPropertyOrder: otherwise ordering used is alphabetic (JDK does not guarantee ordering for fields within class so CSV module defauilts to alphabetic ordering).

@cowtowncoder
Copy link
Member

@zhaoxi2000 I think you have the same problem: your POJO definition does not specify ordering of properties with @JsonPropertyOrder, and the default ordering is alphabetic. That would then map first 5 names into 5 columns, which is not what you want based on header line.

Instead leave out the whole schemaFor(AppsFlyerRawDataBean.class) and instead use .withHeader() when constructing CsvSchema.

@cowtowncoder
Copy link
Member

At this point I don't see actual bug to fix, but I want to help ensure that usage can be changed to work for the use case, so keeping issue open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
csv need-test-case Further progress requires inclusion of problem reproduction, ideally test case
Projects
None yet
Development

No branches or pull requests

3 participants