Skip to content

fix for TIKA-3400 contributed by kamaci#441

Merged
tballison merged 1 commit intoapache:mainfrom
kamaci:TIKA-3400
May 31, 2021
Merged

fix for TIKA-3400 contributed by kamaci#441
tballison merged 1 commit intoapache:mainfrom
kamaci:TIKA-3400

Conversation

@kamaci
Copy link
Copy Markdown
Member

@kamaci kamaci commented May 12, 2021

No description provided.

}
//== is actually substantially faster than .equals(String)
if (typeAtt.type() == UAX29URLEmailTokenizer.TOKEN_TYPES[UAX29URLEmailTokenizer.URL]) {
if (typeAtt.type().equals(UAX29URLEmailTokenizer.TOKEN_TYPES[UAX29URLEmailTokenizer.URL])) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done out of a notional sense of efficiency. I'm not sure we need to change it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, parameter of the TypeAttribute#setType can be exactly that String (UAX29URLEmailTokenizer.TOKEN_TYPES[UAX29URLEmailTokenizer.URL]) ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I think that they had to use enum instead of a string array for such a thing 😊 I'll rollback that lines at my PR.


}
if ((gctxid != ExtendedGUID.nil() ||
if ((!gctxid.equals(ExtendedGUID.nil()) ||
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! We should probably make a static constant ExtendedGUID.NIL to avoid unnecessary object creation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I'm not asking you to do the static thing on this issue. Your catch is important. Thank you!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are welcome!

@kamaci kamaci force-pushed the TIKA-3400 branch 2 times, most recently from a8e0a52 to b294d8f Compare May 21, 2021 06:54
@kamaci
Copy link
Copy Markdown
Member Author

kamaci commented May 21, 2021

@tballison I've updated the PR. Checks fail due to MP4ParserTest.java:101. I don't get same exception at my local environment. Do you have any idea about the reason?

@tballison
Copy link
Copy Markdown
Contributor

I was just able to replicate that in Java 11 on a Mac. ubuntu w Java 8 passes... Ugh... I pushed a simple fix for now.

@kamaci
Copy link
Copy Markdown
Member Author

kamaci commented May 30, 2021

@tballison is there anything left to do for this PR?

@tballison tballison merged commit 42f6e44 into apache:main May 31, 2021
puthurr pushed a commit to puthurr/tika-fork that referenced this pull request Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants