Skip to content

Action and Download URLs should support more protocols #15

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

Closed
rivoduck opened this issue Jun 21, 2016 · 5 comments
Closed

Action and Download URLs should support more protocols #15

rivoduck opened this issue Jun 21, 2016 · 5 comments
Assignees

Comments

@rivoduck
Copy link

ACTION_URL and DOWNLOAD_URL are defined as java.net.URL.URL objects in the getters and setters.
I think they should be set and get as strings in order to accept more protocols than the ones supported by the URL class.
This for example would allow tracking of RTMP URLs.

TODO: verify if Piwik accepts RTMP and other non-HTTP protocols!

Thank you,
Andrea

@bcsorba
Copy link
Collaborator

bcsorba commented Jun 30, 2016

Hi Andrea,

Trying out a few experiments, it seems like Piwik will accept a subset of protocols (including RTMP) for fields such as ACITON_URL, but will not recognize certain other protocols and arbitrary strings. @mattab would you be able to point us to some documentation about what formats are supported by fields such as "url" and "download"? If there is a defined type I would like to accept that as the method parameters, to prevent individuals from submitting unsupported strings and not seeing their results displayed in Piwik.

@rivoduck
Copy link
Author

rivoduck commented Jun 30, 2016

Hi Brett,

I am using your classes to track video streaming access.
I modified your code to deal with URLs as strings to allow for more flexibility.
During my tests I passed URLs with arbitrary strings (eg. RtspServer1:://server1/ and Piwik was able to process and display them without problems.

I modified PiwikRequest.java, here are the methods I changed:

    8a9,11
    > 
    > 
    > 
    123c126
    <     public PiwikRequest(Integer siteId, URL actionUrl){
    ---
    >     public PiwikRequest(Integer siteId, String actionUrl){
    179,180c182,183
    <     public URL getActionUrl(){
    <         return (URL)getParameter(ACTION_URL);
    ---
    >     public String getActionUrl(){
    >         return (String)getParameter(ACTION_URL);
    187c190
    <     public void setActionUrl(URL actionUrl){
    ---
    >     public void setActionUrl(String actionUrl){
    496,497c499,500
    <     public URL getDownloadUrl(){
    <         return (URL)getParameter(DOWNLOAD_URL);
    ---
    >     public String getDownloadUrl(){
    >         return (String)getParameter(DOWNLOAD_URL);
    505c508
    <     public void setDownloadUrl(URL downloadUrl){
    ---
    >     public void setDownloadUrl(String downloadUrl){
    1065,1066c1068,1069
    <     public URL getReferrerUrl(){
    <         return (URL)getParameter(REFERRER_URL);
    ---
    >     public String getReferrerUrl(){
    >         return (String)getParameter(REFERRER_URL);
    1074c1077
    <     public void setReferrerUrl(URL refferrerUrl){
    ---
    >     public void setReferrerUrl(String refferrerUrl){

So far I did not find any problem with Piwik accepting the tracking data.
It is a very impacting change since it requires refactoring the code that uses it!

Andrea

@bcsorba bcsorba self-assigned this Jul 3, 2016
@bcsorba
Copy link
Collaborator

bcsorba commented Jul 4, 2016

All URL based getters and setters now have a get...AsString and a set...WithString methods allowing users to input String values. Overloaded the original getter/setter methods and the constructor was not done to prevent breaking API changes when a "null" value was used.

@mattab
Copy link
Member

mattab commented Jul 8, 2016

. @mattab would you be able to point us to some documentation about what formats are supported by fields such as "url" and "download"? If there is a defined type I would like to accept that as the method parameters, to prevent individuals from submitting unsupported strings and not seeing their results displayed in Piwik.

Hi @bcsorba - our general policy is to try and store as much data as possible, so we do as little validation as possible. Therefore, I think it is best that the SDK forwards all URLs as Strings so that the requests still make it to the Tracking API and it is up to the tracking API to decide what to do. For example, some plugins may change the behavior of what is accepted for a given parameter.
To summarise, the solution you implemented looks good 👍

@mattab mattab reopened this Jul 8, 2016
@mattab mattab closed this as completed Jul 8, 2016
@bcsorba
Copy link
Collaborator

bcsorba commented Jul 30, 2016

Released in v1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants