Commenti e consigli programma

di il
10 risposte

Commenti e consigli programma

Avevo necessita' di fare un programmino per permettere il download di un file via browser e morire una volta scaricato il file.
Ho fatto una cosa del genere e vorrei da voi commenti e consigli su cosa non va e come eventualmente migliorarlo.

Grazie

public class App {

    private static Integer port = 8000;
    private static String filename = "";
    private static Integer count = 1;
    private static Integer counter = 0;
    private static String fname = null;
    private static File file = null;

    public static void main( String[] args ) throws IOException {

        parseArgumentLine(args);

        Path path = Paths.get(filename);
        fname = path.getFileName().toString();

        file = new File(filename);

        if(!file.exists()) {
            System.out.println("The file "+filename+" doesn't exists.");
            exit(-1);
        }
        System.out.println("Now serving on http://localhost:" + port);
        HttpServer server = HttpServer.create(new InetSocketAddress(port), 0);
        server.createContext("/", new MyHandler());
        server.start();

    }

    static class MyHandler implements HttpHandler {
        @Override
        public void handle(HttpExchange t) throws IOException {
            while(counter < count) {
                Headers headers = t.getResponseHeaders();
                headers.add("Content-Type", "application/x-download");
                headers.add("Content-disposition", "attachment; filename=" + fname);
                t.sendResponseHeaders(200, 0);
                OutputStream outputStream = t.getResponseBody();
                Files.copy(file.toPath(), outputStream);
                outputStream.close();
                counter++;
            }

                exit(0);

        }
    }

    private static void parseArgumentLine(String[] args) {
        Options opt = new Options();
        opt.addOption("p", "port", true, "HTTP port");
        opt.addOption("f", "file", true, "File to serve.");
        opt.addOption("c", "count", true, "How many times the file can be downloaded.");
        CommandLineParser parser = new DefaultParser();
        HelpFormatter formatter = new HelpFormatter();

        try {
            CommandLine cmd = parser.parse(opt, args);
            if (!cmd.hasOption("f")) {
                throw new ParseException("A filename is required.");
            }
            if (cmd.hasOption("p")) {
                 try {
                     port = Integer.parseInt(cmd.getOptionValue("p"));
                 } catch (Exception e) {
                     throw new ParseException("Invalid port. This must be a number.");
                 }
            }

            if (cmd.hasOption("c")) {
                try {
                    count = Integer.parseInt(cmd.getOptionValue("c"));
                } catch (Exception e) {
                    throw new ParseException("Invalid port. This must be a number.");
                }
            }

            if (cmd.hasOption("f")) {
                filename = cmd.getOptionValue("f");
            }
        } catch (ParseException exp) {

            System.out.println("Unexpected exception: " + exp.getMessage());

            formatter.printHelp("Woof", opt);
            exit(-1);
        }

    }
}

10 Risposte

  • Re: Commenti e consigli programma

    morellik ha scritto:


    vorrei da voi commenti e consigli su cosa non va e come eventualmente migliorarlo.
    Beh, la prima cosa "ad occhio" è il design generale della classe: tutti campi "statici" e comunicazione tra i vari metodi tramite quei campi. Non è molto buono ... sicuramente non è "object oriented". Anche se poi ovviamente tecnicamente funziona.

    Per il server HTTP stai usando la light-weight HTTP server API del framework standard, vero? (com.sun.net.httpserver)
  • Re: Commenti e consigli programma

    andbin ha scritto:


    morellik ha scritto:


    vorrei da voi commenti e consigli su cosa non va e come eventualmente migliorarlo.
    Beh, la prima cosa "ad occhio" è il design generale della classe: tutti campi "statici" e comunicazione tra i vari metodi tramite quei campi. Non è molto buono ... sicuramente non è "object oriented". Anche se poi ovviamente tecnicamente funziona.
    In effetti e' molto 'funzionale'. Ragionando piu' OOP, come avresti impostato il tutto?

    Per il server HTTP stai usando la light-weight HTTP server API del framework standard, vero? (com.sun.net.httpserver)
    Sì.
  • Re: Commenti e consigli programma

    morellik ha scritto:


    In effetti e' molto 'funzionale'. Ragionando piu' OOP, come avresti impostato il tutto?
    Ad esempio una classe es. FileServerApp con il main e la logica di parsing degli argomenti. E poi un'altra classe es. FileServer che riceve i dati di configurazione (e li tiene in campi di istanza) e incapsula la gestione del server.

    morellik ha scritto:


    Per il server HTTP stai usando la light-weight HTTP server API del framework standard, vero? (com.sun.net.httpserver)
    Sì.
    L'avevo immaginato, vedendo i nomi delle classi e l'handler.

    Mi lascia molto perplesso il while(counter < count) nel handle(). Vuoi ripetere il contenuto del file più volte nella stessa response? ... e che senso ha?
    E pure il exit sempre nel handle che non è molto "bello".
  • Re: Commenti e consigli programma

    andbin ha scritto:


    morellik ha scritto:


    In effetti e' molto 'funzionale'. Ragionando piu' OOP, come avresti impostato il tutto?
    Ad esempio una classe es. FileServerApp con il main e la logica di parsing degli argomenti. E poi un'altra classe es. FileServer che riceve i dati di configurazione (e li tiene in campi di istanza) e incapsula la gestione del server.
    Sara' un buon esercizio per migliorarlo. Ci provero',

    morellik ha scritto:


    Per il server HTTP stai usando la light-weight HTTP server API del framework standard, vero? (com.sun.net.httpserver)
    Sì.
    L'avevo immaginato, vedendo i nomi delle classi e l'handler.

    Mi lascia molto perplesso il while(counter < count) nel handle(). Vuoi ripetere il contenuto del file più volte nella stessa response? ... e che senso ha?
    E pure il exit sempre nel handle che non è molto "bello".
    L'idea era quella di permettere il download count volte e non sapevo come far uscire il programma una volta raggiunto il valore di count.
    Dove potrei mettere il ciclo?
  • Re: Commenti e consigli programma

    morellik ha scritto:


    L'idea era quella di permettere il download count volte e non sapevo come far uscire il programma una volta raggiunto il valore di count.
    Quello che hai fatto non è permettere N download distinti ma 1 solo download con N ripetizioni dello stesso file!!

    morellik ha scritto:


    Dove potrei mettere il ciclo?
    Non c'è da fare alcun ciclo!! Serve mantenere e aggiornare solo uno "stato" (che già fai). Se il count è minore del massimo permesso, fai lo streaming del file nella response come hai fatto, poi incrementi count.
    Alla prossima request farà lo stesso test e se permesso, altro streaming. Se ad un certo punto count non è più minore del massimo, puoi fare diverse cose: inviare uno status 403 ("forbidden") oppure ad esempio stoppare il server.
  • Re: Commenti e consigli programma

    andbin ha scritto:


    morellik ha scritto:


    L'idea era quella di permettere il download count volte e non sapevo come far uscire il programma una volta raggiunto il valore di count.
    Quello che hai fatto non è permettere N download distinti ma 1 solo download con N ripetizioni dello stesso file!!



    morellik ha scritto:


    Dove potrei mettere il ciclo?
    Non c'è da fare alcun ciclo!! Serve mantenere e aggiornare solo uno "stato" (che già fai). Se il count è minore del massimo permesso, fai lo streaming del file nella response come hai fatto, poi incrementi count.
    Alla prossima request farà lo stesso test e se permesso, altro streaming. Se ad un certo punto count non è più minore del massimo, puoi fare diverse cose: inviare uno status 403 ("forbidden") oppure ad esempio stoppare il server.
    Scusa ma sono diversamente giovane e sto provando a mettere questo controllo, ma non funziona ovunque lo metta. Mi dici esattamente dove lo devo mettere prima che mi prenda un accidente?

    Grazie
  • Re: Commenti e consigli programma

    morellik ha scritto:


    sto provando a mettere questo controllo, ma non funziona ovunque lo metta. Mi dici esattamente dove lo devo mettere prima che mi prenda un accidente?
    Devi mantenere delle variabili in modo che abbiano una "durata" tale per cui mantengano il valore tra una invocazione di handle(HttpExchange) e l'altra.
    Questo lo fai già, sono le due variabili count/counter che hanno durata molto lunga avendole messe come campi static di App.

    Quindi nel handle(HttpExchange) semplicemente (pseudo codice):
    IF counter minore di count
    THEN
        ....streaming del file nella response
        incrementa counter
    ELSE
        ...qui puoi inviare un certo status code oppure addirittura stoppare il serve
  • Re: Commenti e consigli programma

    Una cosa del genere?
    
    static class MyHandler implements HttpHandler {
            @Override
            public void handle(HttpExchange t) throws IOException {
                if(counter < count) {
                    Headers headers = t.getResponseHeaders();
                    headers.add("Content-Type", "application/x-download");
                    headers.add("Content-disposition", "attachment; filename=" + fname);
                    t.sendResponseHeaders(200, 0);
                    OutputStream outputStream = t.getResponseBody();
                    Files.copy(file.toPath(), outputStream);
                    outputStream.close();
                    counter++;
                } else {
                    server.stop(0);
                }
    
            }
        }
    
    
    
  • Re: Commenti e consigli programma

    morellik ha scritto:


    Una cosa del genere?
    Sì, esattamente, ma a patto che quella variabile server abbia durata e visibilità tali per cui sia usabile dal handle (nel tuo codice iniziale la variabile server era locale al main e quindi NON "visibile" da MyHandler).
  • Re: Commenti e consigli programma

    Infatti l'ho messa tra quelle private static. Alla fine un po' un abbriccico anche se funzionante.
    Ora vedo se riesco e renderlo OOP seguendo i tuoi suggerimenti.

    Grazie per ora.
Devi accedere o registrarti per scrivere nel forum
10 risposte