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

Support data URLs in the HTTP request factory #1502

Merged
merged 12 commits into from Oct 7, 2020
Merged

Conversation

gberaudo
Copy link
Contributor

No description provided.

@gberaudo
Copy link
Contributor Author

@sbrunner, I added data:url support to the image layer (and others based on the same http request mechanism).

I tested this with some png data:url and it passed:

  • the static image gets decoded and printed;
  • it is composed by the print into the background raster layer.

What do you think?

@sbrunner
Copy link
Member

sbrunner commented Oct 1, 2020

It shouldn't use a handler as it's done here?
https://github.com/mapfish/mapfish-print/pull/1359/files?diff=unified&w=1

It should be tested throw an example...

@gberaudo
Copy link
Contributor Author

gberaudo commented Oct 1, 2020

@sbrunner, the ImageLayer and associated factories are using URIs. An Handle only works with URLs.

https://github.com/mapfish/mapfish-print/blob/master/core/src/main/java/org/mapfish/print/map/image/ImageLayer.java#L94-L95

How do you handle it?

@sbrunner
Copy link
Member

sbrunner commented Oct 1, 2020

I'm curious to know why it's an URI, but it's another story...

@gberaudo
Copy link
Contributor Author

gberaudo commented Oct 1, 2020

@sbrunner, the code is now making use of the Handler. Does it look better now?

@sbrunner
Copy link
Member

sbrunner commented Oct 1, 2020

Yes :-), and the changes in DataUrlConnection.java can be canceled :-)

@gberaudo
Copy link
Contributor Author

gberaudo commented Oct 1, 2020

Yes :-), and the changes in DataUrlConnection.java can be canceled :-)

Indeed @sbrunner, I forgot about it. It is done now.

Copy link
Member

@sbrunner sbrunner left a comment

Choose a reason for hiding this comment

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

Thanks :-)

@gberaudo gberaudo force-pushed the support_data_urls branch 2 times, most recently from 439d360 to 81a12bc Compare October 6, 2020 12:52
@gberaudo gberaudo changed the base branch from no-nowait-lock-320 to 3.20 October 6, 2020 15:01
@gberaudo
Copy link
Contributor Author

gberaudo commented Oct 6, 2020

@sbrunner, I changed the base to 3.20 so the build should soon pass.
OK for you to merge this in the 3.20 branch and then backport it to master?

@gberaudo gberaudo changed the base branch from 3.20 to master October 7, 2020 07:30
@gberaudo
Copy link
Contributor Author

gberaudo commented Oct 7, 2020

@sbrunner, as discussed, I rebased this branch on top of master.

@gberaudo gberaudo merged commit 2bce19d into master Oct 7, 2020
@gberaudo gberaudo deleted the support_data_urls branch October 7, 2020 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants