A bug in CachedDataService | (i don't know where to post issue)

[Fabrice Kabongo]

when you create a CachedData (e.g. CachedData cd;), 

you set the url, you add want to update it with CachedDataService.update(cd,new ActionListener(){//what ever}).

you'll receive a good :

java.lang.IllegalStateException: URL is null
 at com.codename1.io.ConnectionRequest.validateImpl(ConnectionRequest.java:1216)
 at com.codename1.io.NetworkManager.addToQueue(NetworkManager.java:608)
 at com.codename1.io.NetworkManager.addToQueue(NetworkManager.java:563)
 at com.codename1.io.services.CachedDataService.updateData(CachedDataService.java:77)
 at userclasses.StateMachine.onActualite_ActualiteActualiserAction(StateMachine.java:322)
 at generated.StateMachineBase.handleComponentAction(StateMachineBase.java:1714)

so i opened your CachedDataService class (netbean offers that)

i saw that :

public static void updateData(CachedData d, ActionListener callback)
 {
        if(d.isFetching())
       {
            return;
        }
        d.setFetching(true);
        CachedDataService c = new CachedDataService();
        c.setPost(false);
        if(callback != null) {
            c.addResponseListener(callback);
        }
        if(d.getModified() != null && d.getModified().length() > 0) {
            c.addRequestHeader("If-Modified-Since", d.getModified());
            c.addRequestHeader("If-None-Match", d.getEtag());
            //you forgot to add : c.setUrl(d.getUrl);
        }
        NetworkManager.getInstance().addToQueue(c);        
    }

i hope you'll fix that very fast. and please can you tell where i can post a issue

P.S. : i've a up to date version. i've made my update yesterday

[Fabrice Kabongo]

something else. when the cacheddataservice get the data from the network it does'nt give it to the cachedData! i think you do like this:

//the new update method
public static void updateData(ReseauCachedData d, ActionListener callback) {
        if(d.isFetching()) {
            return;
        }
        d.setFetching(true);
        ReseauCachedDataService c = new ReseauCachedDataService();
        c.data = d;

        c.setPost(false);
        if(callback != null)
        {
            c.addResponseListener(callback);
        }
        if(d.getModified() != null && d.getModified().length() > 0)
        {
            c.addRequestHeader("If-Modified-Since", d.getModified());
            c.addRequestHeader("If-None-Match", d.getEtag());
        }

            c.setUrl(d.getUrl());
        NetworkManager.getInstance().addToQueue(c);        
    }

i've just create my own CachedData and CachedDataService for test and this changing update method by this one fix all the problem. so i hope you change it and use mine. and you use if don't forget to add a :

/**
* thanks to Fabrice Kabongo
*/

 i'll be so proud of me! lol

you've made a very good job guys

[Fabrice Kabongo]

sorry for my bad way of writting but i'm a french speaker

[Shai Almog]

Thanks, I fixed the URL in our implementation.

[Fabrice Kabongo]

Sorry, but i've find something else.

your implementation:

public static void updateData(ReseauCachedData d, ActionListener callback) {
        if(d.isFetching()) {
            return;
        }
        d.setFetching(true);

        ReseauCachedDataService c = new ReseauCachedDataService();

        c.setPost(false);
        if(callback != null)
        {
            c.addResponseListener(callback);
        }
        if(d.getModified() != null && d.getModified().length() > 0)
        {
            c.addRequestHeader("If-Modified-Since", d.getModified());

            c.addRequestHeader("If-None-Match", d.getEtag());//generates a null pointer exception when the server didn't set the ETag
        }
            c.setUrl(d.getUrl());
        NetworkManager.getInstance().addToQueue(c);        
    }

the my correction:

public static void updateData(ReseauCachedData d, ActionListener callback) {
        if(d.isFetching()) {
            return;
        }
        d.setFetching(true);

        ReseauCachedDataService c = new ReseauCachedDataService();

        c.setPost(false);
        if(callback != null)
        {
            c.addResponseListener(callback);
        }
        if(d.getModified() != null && d.getModified().length() > 0)
        {
            c.addRequestHeader("If-Modified-Since", d.getModified());

            //start of the correction
            if(d.getEtag()!=null)
            {
                c.addRequestHeader("If-None-Match", d.getEtag());
            }
            //and of the correction
        }
            c.setUrl(d.getUrl());
        NetworkManager.getInstance().addToQueue(c);        
    }

[Shai Almog]

Thanks, for the fix.

[Fabrice Kabongo]

When the updateData method is invoked the CacheData is locked:

if(d.isFetching()) {
            return;
        }
d.setFetching(true);

 

and when the network thread read the reponse(in the case that the statut code is 200) it set a another CachedData,

this one:

public class CachedDataService extends ConnectionRequest
{
    private CachedData data = new CachedData();//this one

here:

protected void readResponse(InputStream input) throws IOException  
    {
        byte[] b = Util.readInputStream(input);
        data.setData(b);//here
        Log.p(String.valueOf(data.getData().length));
        fireResponseListener(new NetworkEvent(this, data));//and it sent to the user
    }

the who get it like this:

listeBreves.setUrl("http://localhost/web/breves/?token="+token);
CachedDataService.updateData(listeBreves, new ActionListener()
{
    public void actionPerformed(ActionEvent evt)
    {
        DataOutputStream dos = null;
        NetworkEvent nvt = (NetworkEvent)evt;
        listeBreves = (CachedData)nvt.getMetaData();//here
    }
});

But if the statut code is not 200 or 301, the lock never set to false, so you'll not be able to re-use the updateData method with the same CachedData object.

 

protected void handleErrorResponseCode(int code, String message)
{
        //miss a data.setFetching(false);
        // but even if we do that is not the original CachedData that'll be unlocked
        if(code == 304) {
            // data unmodified
            return;
        }
        super.handleErrorResponseCode(code, message);
    }

so i propose this:

public class CachedDataService extends ConnectionRequest {
    private CachedData data = new CachedData();
   
    private CachedDataService() {}


    /**
     * Makes sure the cached data class is properly registered as an externalizable. This must
     * be invoked for caching to work
     */
    public static void register() {        
        Util.register("CachedData", CachedData.class);
    }
   
    /**
     * Checks that the cached data is up to date and if a newer version exits it updates the data in place
     *
     * @param d the data to check
     * @param callback optional callback to be invoked on request completion
     */

    public static void updateData(CachedData d, ActionListener callback) {
        if(d.isFetching()) {
            return;
        }
        d.setFetching(true);
        CachedDataService c = new CachedDataService();

        c.data = d;//to ensure that we store a link to the original CachedData

        c.setPost(false);
        if(callback != null)
        {
            c.addResponseListener(callback);
        }
        if(d.getModified() != null && d.getModified().length() > 0)
        {
            c.addRequestHeader("If-Modified-Since", d.getModified());

            if(d.getEtag()!=null)
            {//already added
                c.addRequestHeader("If-None-Match", d.getEtag());
            }
        }
            c.setUrl(d.getUrl());//already added too
        NetworkManager.getInstance().addToQueue(c);        
    }
   
    /**
     * @inheritDoc
     */
    @Override
    protected void handleErrorResponseCode(int code, String message)
    {
        data.setFetching(false);//the first correction
        if(code == 304) {
            // data unmodified
            return;
        }
        super.handleErrorResponseCode(code, message);
    }




    /**
     * @inheritDoc
     */
    @Override
    protected void readHeaders(Object connection) throws IOException
    {
        String last = getHeader(connection, "Last-Modified");
        String etag = getHeader(connection, "ETag");
        if(last != null && last.length() > 0) {
            data.setModified(last);
            data.setEtag(etag);
        }
        //i would like to add this here: data.setFetching(false);
               //instead of everywere but it will be to soon to unlock the CachedData 

    }


    /**
     * @inheritDoc
     */
    @Override
    protected void readResponse(InputStream input) throws IOException  
    {
        byte[] b = Util.readInputStream(input);
        data.setData(b);
        Log.p(String.valueOf(data.getData().length));
        data.setFetching(false);//so we also add here because "data" is no more a new CacheData
        fireResponseListener(new NetworkEvent(this, data));
    }

what do you think about this? is there something wrong so that i'll modify my CachedDataService class?

[Shai Almog]

Thanks, for the suggestion. I'll fix these.
The read headers isn't a good place since error codes and exceptions won't be handled.