Patrick,
Maybe you need to be clear in the problem statement. I can't actually
find it anywhere in this email thread. I'm reverse engineering to it as
follows:
You got an IPPPrintService for lookup via
PrintServiceLookup.lookupPrintServices(null, null)
but a UnixPrintService via
attributes.add(new PrinterName(name, null));
PrintServiceLookup.lookupPrintServices(null, attributes)
If you read my email you should see that if you make the change
I proposed you will never enter the getNamedPrinterXXX() methods
if you are using CUPS, which seems like it should fix your problem
of different classes.
You don't really need to test the class and probably shouldn't.
service.equals(serviceByName) seems better.
And although getServiceByName() shouldn't enumerate all printers,
because that can be very costly in some configurations,
it likely should first check to see if we already enumerated all printers,
and then see if its in the list. As its written its really intended for the
case where no enumeration has been performed but if it already
happened there's no harm in leveraging that.
Put another, way does this patch (on its own, no other changes) fix your
problem
It modifies the getPrinterByName() method. If not, why, not, and please
print
out the actual class/service names seen by your test so we can see more
clearly
diff --git a/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
b/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
--- a/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
+++ b/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
@@ -366,6 +366,29 @@
if (name == null || name.equals("") || !checkPrinterName(name)) {
return null;
}
+
+ if (printServices != null) {
+ for (int i=0; i<printServices.length; i++) {
+ if (name.equals(printServices[i].getName())) {
+ return printServices[i];
+ }
+ }
+ }
+
+ if (CUPSPrinter.isCupsRunning()) {
+ try {
+ URL url = new URL("http://"+
+ CUPSPrinter.getServer()+":"+
+ CUPSPrinter.getPort()+"/"+
+ name);
+ printer = new IPPPrintService(name, url);
+ } catch (Exception e) {
+ }
+ if (printer != null) {
+ return printer;
+ }
+ }
+
if (isMac() || isSysV()) {
printer = getNamedPrinterNameSysV(name);
} else {
BTW keep all revisions of your code around so that people can compare.
I have no reference to what the earlier version of your fix did.
-phil.
On 5/8/2013 12:48 AM, Patrick Reinhart wrote:
Hi Phil,
Sorry, I can not share your point of the methods
getNamedPrinterNameXX() always returning a UnixPrintService though.
That is exactly the point of inconsistency shown by the test case. As
far as I checked it the only referred places of those
getNamedPrinterNameXX() is exactly the getServiceByName() method
mentioned to place the fix.
In my opinion those method names are misleading anyway. They both
called getNamedPrinterNameXX() even though they return a PrintService.
As I assume that they originally did return the active printer name
and the PrintService was created outside, as the usage of the
getAllPrinterNamesXX() methods - that are called when creating all
PrintService instances. You explained me before that in case of
hundreds of printers it's faster to just lookup the printer state of
one printer only - which is done within the getNamedPrinterNameXX()
methods.
Patrick
Am 08.05.13 01:38, schrieb Phil Race:
I am assuming this current webrev replaced the previous
one:- http://reinharts.dyndns.org/webrev/
getNamedPrinterNameSysV() and
getNamedPrinterNameBSD()
should not be changed as they should always
create a UnixPrintService and so can do this
directly. You are subverting the code in
there to first check for CUPS which shouldn't
be needed.
I haven' t tested this out but from inspection,
I think the problem is in getServiceByName() where
it is ignorant of the CUPS possibility.
It needs to have the isCUPSRunning() check and
if so create an IPPPrintService().
So that's where you should place your call.
That is likely the only change here that is really necessary.
The refactoring is OK but but not essential to the fix.
-phil.
On 5/7/2013 2:23 AM, Patrick Reinhart wrote:
Hi Jennifer,
I have changed the test to not use non internal packages now that
produces the same results for my case now.
Now I still waiting for the feedback of Phil to get that fixed
correctly..
Best regards
Patirck
Quoting Jennifer Godinez <jennifer.godi...@oracle.com>:
Yes I have and dicussed with Phil. It looks pretty good but there
may be a safer way to fix it since the fix is still using
lpc/lpstat commands for CUPS. Also, the regression test should be
modified to use non internal package. Phil will give his input on
this too.
Jennifer
