views:

56

answers:

5

Hi, I'm trying to return an XML file based on my query results. I'm very new to this so I'm not really sure where I'm going wrong. Is this a realistic way to go about doing this or is there something simpler? Right now I'm getting these exceptions:

Error performing query: javax.servlet.ServletException: org.xml.sax.SAXParseException: Content is not allowed in prolog.

If I run my query in isql*plus, it does execute

import java.io.*;
import java.util.*;
import java.sql.*;           // JDBC packages
import javax.servlet.*;
import javax.servlet.http.*;
import javax.xml.parsers.*;
import org.xml.sax.*;
import org.xml.sax.helpers.*;

public class step5 extends HttpServlet {

   public static final String DRIVER = "sun.jdbc.odbc.JdbcOdbcDriver";
   public static final String URL = "jdbc:odbc:rreOracle";
   public static final String username = "cm485a10";
   public static final String password = "y4e8f7s5";

   SAXParserFactory factory;

   public void init() throws ServletException {

      factory = SAXParserFactory.newInstance();
   }

   public void doGet (HttpServletRequest  request,
                      HttpServletResponse response)
      throws ServletException, IOException
      {
      PrintWriter out = response.getWriter();
      Connection con = null;

      try {

            Class.forName(DRIVER);

            con = DriverManager.getConnection(URL,username,password);

            try {
               Statement stmt = con.createStatement();
               ResultSet rs = stmt.executeQuery("SELECT sale_id, home_id, agent_id, customer_id FROM sale");

               String xml = "";
               xml = xml + "<sales_description>";
               xml = xml + "<sale>";

               boolean courseDataDone = false;
               while (rs.next()) {
                  String sale_id = rs.getString(1);
                  String home_id = rs.getString(2);
                  String agent_id = rs.getString(3);
                  String customer_id = rs.getString(4);

                  if (!courseDataDone) {
                     xml = xml + "<sale_id>" + sale_id + "</sale_id>" +
                                 "<home_id>" + home_id + "</home_id>" +
                                 "<agent_id>" + agent_id + "</agent_id>" +
                                 "<customer_id>" + customer_id + "</customer_id>" +
                                 "" +
                                 "";
                     courseDataDone = true;
                  }

               }

               xml = xml + "</sale>" +
                           "</sales_description>";

               try {
                  SAXParser parser = factory.newSAXParser();
                  InputSource input = new InputSource(new StringReader(xml));
                  parser.parse(input, new DefaultHandler());
               } catch (ParserConfigurationException e) {
                  throw new ServletException(e);
               } catch (SAXException e) {
                  throw new ServletException(e);
               }

               response.setContentType("text/xml;charset=UTF-8");
               out.write(xml);

            } catch(Exception ex) {
               out.println("Error performing query: " + ex);
               con.close();
               return;
            }

         } catch(Exception ex) {
            out.println("Error performing DB connection: " + ex);
            return;
         }

   }
}

Any help/tips would be appreciated.

+2  A: 

What are you trying to accomplish here? This code looks very confusing for several reasons:

  1. You're presumably trying to build up an XML string, but you're not appending any XML tags to it at all.
  2. There's a lot of no-ops in there, such as xml = xml + ""; which doesn't achieve anything.
  3. I'm not 100% sure what you want to achieve in the try block near the end. This block will have the side-effect of ensuring your xml string is valid XML, but if this is what you want to do there are probably clearer (and more efficient) ways of validating. If you're hoping it will magically transform your String into XML, then it won't (in fact no matter what, it can't modify the contents of the xml variable so this would be a no-op.

Perhaps it would help if you talked through what you're trying to do here, with particular reference to what you expect the state of affairs to be at each stage. Right now, you're building up a string that looks something like:

FirstSaleIDFirstHomeFirstAgentFirstCustomerSecondSaleIDSecondHomeSecondAgentSecondCustomer...

Then you try to parse this as XML. As you might expect, this is not valid XML hence the parser throws the error (in particular "no content in prolog" means that you have character data before the first tag definition).

I would give you advice on how to improve this but I really have no idea what you expect this to do...

Andrzej Doyle
Thanks for the response. Fair enough. What I'm trying to accomplish is fairly simple, I think. Basically all I want to do is run a query and then based on the results...return a valid XML to the browser. From your response, I can tell I'm way off base so far.
relyt
+3  A: 

One tip would be layering your code a bit more. Servlets shouldn't be importing from java.sql. Put that code in a separate class, test it, and let your servlet call its methods.

You're creating XML in the most brain dead way possible by concatentating strings that way. Why not use a library like JDOM or at least a StringBuilder?

And skaffman's right: your code makes no sense otherwise.

Here are a few ideas you can think about for layering. Start with a model object for Sale - after all, Java's an object-oriented language:

package badservlet.model;

public class Sale
{
    private String saleId;
    private String homeId;
    private String agentId;
    private String customerId;

    public Sale(String saleId, String homeId, String agentId, String customerId)
    {
        if ((saleId == null) || (saleId.trim().length() == 0)
            throw new IllegalArgumentException("sales id cannot be blank or null");
        if ((homeId == null) || (homeId.trim().length() == 0)
            throw new IllegalArgumentException("home id cannot be blank or null");
        if ((agentId == null) || (agentId.trim().length() == 0)
            throw new IllegalArgumentException("agent id cannot be blank or null");
        if ((customerId == null) || (customerId.trim().length() == 0)
            throw new IllegalArgumentException("customer id cannot be blank or null");

        this.saleId = saleId;
        this.homeId = homeId;
        this.agentId = agentId;
        this.customerId = customerId;
    }

    public String getSaleId()
    {
        return saleId;
    }

    public String getHomeId()
    {
        return homeId;
    }

    public String getAgentId()
    {
        return agentId;
    }

    public String getCustomerId()
    {
        return customerId;
    }

    @Override
    public String toString()
    {
        return "Sale{" +
               "saleId='" + saleId + '\'' +
               ", homeId='" + homeId + '\'' +
               ", agentId='" + agentId + '\'' +
               ", customerId='" + customerId + '\'' +
               '}';
    }
}

For persistence, start with a DAO interface:

package badservlet.persistence;

import badservlet.model.Sale;

import java.sql.SQLException;
import java.util.List;

public interface SaleDao
{
    List<Sale> find() throws SQLException;
}

And its implementation:

package badservlet.persistence;

import badservlet.model.Sale;

import javax.sql.DataSource;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.List;

public class SaleDaoImpl implements SaleDao
{
    private static final String SELECT_ALL_SQL = "SELECT sale_id, home_id, agent_id, customer_id FROM sale";

    private Connection connection;

    public SaleDaoImpl(Connection connection)
    {
        this.connection = connection;
    }

    public SaleDaoImpl(DataSource dataSource) throws SQLException
    {
        this(dataSource.getConnection());
    }

    public List<Sale> find() throws SQLException
    {
        List<Sale> allSales = new ArrayList<Sale>();

        Statement st = null;
        ResultSet rs = null;

        try
        {
            st = this.connection.createStatement();
            rs = st.executeQuery(SELECT_ALL_SQL);
            while (rs.next())
            {
                String saleId = rs.getString("sale_id");
                String homeId = rs.getString("home_id");
                String agentId = rs.getString("agent_id");
                String customerId = rs.getString("customer_id");
                Sale sale = new Sale(saleId, homeId, agentId, customerId);
                allSales.add(sale);
            }

        }
        catch (SQLException e)
        {
            e.printStackTrace();
        }
        finally
        {
            try { if (rs != null) rs.close(); } catch (SQLException e) { e.printStackTrace(); }
            try { if (st != null) st.close(); } catch (SQLException e) { e.printStackTrace(); } 
        }

        return allSales;
    }
}

And an object-to-XML unmarshaller:

package badservlet.xml;

import badservlet.model.Sale;
import org.jdom.Document;
import org.jdom.Element;
import org.jdom.transform.JDOMResult;

import javax.xml.bind.JAXBException;
import javax.xml.transform.Result;
import java.util.List;

public class SaleUnmarshaller
{
    public void unmarshal(Object object, Result xml) throws JAXBException
    {
        List<Sale> allSales = (List<Sale>) object;
        Document document = new  Document(new Element("sales"));
        for (Sale sale : allSales)
        {
            Element child = new Element("sale");
            child.setAttribute("id", sale.getSaleId());
            child.addContent(new Element("home", sale.getHomeId()));
            child.addContent(new Element("agent", sale.getAgentId()));
            child.addContent(new Element("customer", sale.getCustomerId()));
            document.addContent(child);
        }

        JDOMResult result = new JDOMResult();
        result.setDocument(document);
        xml = result;
    }
}

Let your servlet instantiate these objects and call their methods.

It might look more complicated - more classes than just one - but you've accomplished two things: you've broken your problem down into smaller pieces, and you can test them separately.

duffymo
+1 for pointing out the missing layers in code.
Tushar Tarkas
+1 for the effort, duffy :) Btw: that SQLException catch in `find()` can IMO be left out.
BalusC
Yes it can; thanks for checking, BalusC. (You must be a fine man to have around when doing code review.)
duffymo
A: 

If we assume that this values you retrive form database

|sale_id | home_id | agent_id | customer_id |
|   1    |    10   |  100     |   1000      | 
|   2    |    20   |  200     |   2000      | 
|   3    |    30   |  300     |   3000      | 

the xml a the end look like this

1010100100020003000

And after all You are tying to create a XML file from this.

What You should read about:

First about the ResultSet, i really doubt that those id are string not numbers. next the class StringBuilder, the way you concat strings is wrong because strings is inmutable. And for sure you should look about the Java API for XML

Vash
+1  A: 

Rather than using String concatanation for building XML, Trying using XML API's like DOM, JDOM etc.

Few Tutorial links:

http://www.w3schools.com/dom/default.asp

http://www.xul.fr/en/dom/

http://swik.net/JDOM+Tutorial

http://www.ibm.com/developerworks/java/library/j-jdom/

YoK
+1  A: 

You're missing the prolog. Add this to beginning of your XML:

<?xml version="1.0" encoding="UTF-8"?>

By the way, you don't need the SAX parser here. You aren't modifying the XML at all. Get rid of the parser and just write xml directly to the response. You are also not handling JDBC resources correctly in the finally. Here's a basic example of the improvement:

response.setContentType("text/xml;charset=UTF-8");
PrintWriter writer = response.getWriter();
writer.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
writer.append("<sales_description>");

Connection connection = null;
Statement statement = null;
ResultSet resultSet = null;

try {
    connection = database.getConnection();
    statement = connection.createStatement();
    resultSet = statement.executeQuery("SELECT sale_id, home_id, agent_id, customer_id FROM sale");

    if (resultSet.next()) {
        writer.append("<sale>");
        writer.append("<sale_id>").append(resultSet.getString("sale_id")).append("</sale_id>");
        writer.append("<home_id>").append(resultSet.getString("home_id")).append("</home_id>");
        writer.append("<agent_id>").append(resultSet.getString("agent_id")).append("</agent_id>");
        writer.append("</sale>");
    }
} catch (SQLException e) {
    // Handle.
} finally { 
    if (resultSet != null) try { resultSet.close(); } catch (SQLException logOrIgnore) {}
    if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
    if (connection != null) try { connection.close(); } catch (SQLException logOrIgnore) {}
}

writer.append("</sales_description>");

To write all records, just replace if (resultSet.next()) by while (resultSet.next()).

To handle the exception more gracefully, i.e. throwing an ServletException which ends in an error page instead of a halfbaked XML, you'd like to build the XML using StringBuilder. Just replace PrintWriter by new StringBuilder() and then at end, do response.getWriter().write(builder.toString()).

BalusC
+1 - That's some tight code.
duffymo