You are viewing a plain text version of this content. The canonical link for it is here.
Posted to slide-dev@jakarta.apache.org by re...@apache.org on 2001/04/25 03:56:14 UTC
cvs commit: jakarta-slide/src/share/org/apache/slide/content ContentImpl.java NodeRevisionDescriptor.java
remm 01/04/24 18:56:14
Modified: src/share/org/apache/slide/content ContentImpl.java
NodeRevisionDescriptor.java
Log:
- Enhance handling of displayname.
Revision Changes Path
1.24 +9 -5 jakarta-slide/src/share/org/apache/slide/content/ContentImpl.java
Index: ContentImpl.java
===================================================================
RCS file: /home/cvs/jakarta-slide/src/share/org/apache/slide/content/ContentImpl.java,v
retrieving revision 1.23
retrieving revision 1.24
diff -u -r1.23 -r1.24
--- ContentImpl.java 2001/03/21 16:47:27 1.23
+++ ContentImpl.java 2001/04/25 01:56:14 1.24
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-slide/src/share/org/apache/slide/content/ContentImpl.java,v 1.23 2001/03/21 16:47:27 juergen Exp $
- * $Revision: 1.23 $
- * $Date: 2001/03/21 16:47:27 $
+ * $Header: /home/cvs/jakarta-slide/src/share/org/apache/slide/content/ContentImpl.java,v 1.24 2001/04/25 01:56:14 remm Exp $
+ * $Revision: 1.24 $
+ * $Date: 2001/04/25 01:56:14 $
*
* ====================================================================
*
@@ -77,7 +77,7 @@
* Implementation of the content interface.
*
* @author <a href="mailto:remm@apache.org">Remy Maucherat</a>
- * @version $Revision: 1.23 $
+ * @version $Revision: 1.24 $
*/
public final class ContentImpl implements Content {
@@ -424,7 +424,11 @@
revisionDescriptor.setCreationDate(new Date());
}
// set the display name (in case of copy)
- revisionDescriptor.setProperty("displayname", strUri);
+ String resourceName = strUri;
+ int lastSlash = resourceName.lastIndexOf('/');
+ if (lastSlash != -1)
+ resourceName = resourceName.substring(lastSlash + 1);
+ revisionDescriptor.setName(resourceName);
Uri objectUri = namespace.getUri(strUri);
1.17 +5 -5 jakarta-slide/src/share/org/apache/slide/content/NodeRevisionDescriptor.java
Index: NodeRevisionDescriptor.java
===================================================================
RCS file: /home/cvs/jakarta-slide/src/share/org/apache/slide/content/NodeRevisionDescriptor.java,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -r1.16 -r1.17
--- NodeRevisionDescriptor.java 2001/03/06 03:19:57 1.16
+++ NodeRevisionDescriptor.java 2001/04/25 01:56:14 1.17
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-slide/src/share/org/apache/slide/content/NodeRevisionDescriptor.java,v 1.16 2001/03/06 03:19:57 remm Exp $
- * $Revision: 1.16 $
- * $Date: 2001/03/06 03:19:57 $
+ * $Header: /home/cvs/jakarta-slide/src/share/org/apache/slide/content/NodeRevisionDescriptor.java,v 1.17 2001/04/25 01:56:14 remm Exp $
+ * $Revision: 1.17 $
+ * $Date: 2001/04/25 01:56:14 $
*
* ====================================================================
*
@@ -81,7 +81,7 @@
* Node Revision Descriptor class.
*
* @author <a href="mailto:remm@apache.org">Remy Maucherat</a>
- * @version $Revision: 1.16 $
+ * @version $Revision: 1.17 $
*/
public final class NodeRevisionDescriptor implements Serializable, Cloneable {
@@ -542,7 +542,7 @@
* @param name New name
*/
public void setName(String name) {
- setProperty(NAME, name, true);
+ setProperty(NAME, "<![CDATA[" + name + "]]>", true);
}
Re: cvs commit: jakarta-slide/src/share/org/apache/slide/content ContentImpl.java NodeRevisionDescriptor.java
Posted by Remy Maucherat <re...@apache.org>.
> > > My approach doesn't work (at least with microsoft web-folders). This
is
> > > because it applies to all the properties, and some of these are
> > > apparently treated specially. Having the resourcetype property not be
> > > <collection/> (exactly. Putting it in a CDATA section is too big a
> > > change, apparently) for a directory seems to break MS web-folders.
> >
> > I tried that before (use writeData everywhere), but it's not a good idea
to
> > do that when the property value is an XML node.
>
> Yes, so I found. Basically, slide tries to be extremely general all the
> way through (which is definately a good thing!), but we run into
> problems once we have to interface with the outside world - all
> surmountable problems, but it's hard to catch every point where extra
> processing is required.
No big surprise, I suppose ... Integration is always the main problem ;-)
> Hrmm. Yes, that sounds like a nice solution (though it's not exactly
> trivial to implement in absolute generality, it's not too hard to catch
> all the cases where it's ever likely to be an issue (unbalanced tags,
> loose special characters, etc, without bothering to check every little
> detail - that'd be too slow).
Agreed.
> If I have time soon (unfortunately highly unlikely - I'm snowed under
> with all sorts of things), I'll do this, but for now your current
> solution works.
Remy
Re: cvs commit: jakarta-slide/src/share/org/apache/slide/content
ContentImpl.java NodeRevisionDescriptor.java
Posted by Michael Smith <ms...@xn.com.au>.
> > My approach doesn't work (at least with microsoft web-folders). This is
> > because it applies to all the properties, and some of these are
> > apparently treated specially. Having the resourcetype property not be
> > <collection/> (exactly. Putting it in a CDATA section is too big a
> > change, apparently) for a directory seems to break MS web-folders.
>
> I tried that before (use writeData everywhere), but it's not a good idea to
> do that when the property value is an XML node.
Yes, so I found. Basically, slide tries to be extremely general all the
way through (which is definately a good thing!), but we run into
problems once we have to interface with the outside world - all
surmountable problems, but it's hard to catch every point where extra
processing is required.
>
> More generally, WebDAV makes the implicit requirement that a property value
> is valid XML. I would be considering adding (and enforcing) this requirement
> for Slide.
Well, I just looked. It's actually an explicit requirement (section 4.4
of the RFC) that property values be well formed (but not 'valid', of
course - I guess you were using that in the english-word sense rather
than the technical-xml sense. Confusing language ;-)
>
> I think we can come up with something nice eventually.
> Here's an option : setProperty could parse the property value to check if
> it's a valid XML fragment, and wrap the value with a CDATA if it turns out
> it's not.
Hrmm. Yes, that sounds like a nice solution (though it's not exactly
trivial to implement in absolute generality, it's not too hard to catch
all the cases where it's ever likely to be an issue (unbalanced tags,
loose special characters, etc, without bothering to check every little
detail - that'd be too slow).
If I have time soon (unfortunately highly unlikely - I'm snowed under
with all sorts of things), I'll do this, but for now your current
solution works.
Michael
Re: cvs commit: jakarta-slide/src/share/org/apache/slide/content ContentImpl.java NodeRevisionDescriptor.java
Posted by Remy Maucherat <re...@apache.org>.
> > The fix then becomes removing this change, and changing a pair of
> > writeText() calls to writeData(), in PropFindMethod.java (the logical
> > place - the slide core shouldn't know about the output format of the
> > data, and doesn't in other places). If you agree, I'll commit this
> > change (just doing some testing on it now).
>
> A followup to my own mail, after some quick testing:
>
> My approach doesn't work (at least with microsoft web-folders). This is
> because it applies to all the properties, and some of these are
> apparently treated specially. Having the resourcetype property not be
> <collection/> (exactly. Putting it in a CDATA section is too big a
> change, apparently) for a directory seems to break MS web-folders.
I tried that before (use writeData everywhere), but it's not a good idea to
do that when the property value is an XML node.
More generally, WebDAV makes the implicit requirement that a property value
is valid XML. I would be considering adding (and enforcing) this requirement
for Slide.
I think we can come up with something nice eventually.
Here's an option : setProperty could parse the property value to check if
it's a valid XML fragment, and wrap the value with a CDATA if it turns out
it's not.
> So I see that the problem is a bit bigger. Your way works, but seems
> rather like an ugly hack to me. If we can come up with a better
> solution, that'd be good. That would probably end up having to be a
> special treatment of displayname (to put it in a CDATA section) in
> PropFindMethod.java
>
> For now, I'll leave things as-is, and leave the decision up to you.
Remy
Re: cvs commit: jakarta-slide/src/share/org/apache/slide/content
ContentImpl.java NodeRevisionDescriptor.java
Posted by Michael Smith <ms...@xn.com.au>.
> The fix then becomes removing this change, and changing a pair of
> writeText() calls to writeData(), in PropFindMethod.java (the logical
> place - the slide core shouldn't know about the output format of the
> data, and doesn't in other places). If you agree, I'll commit this
> change (just doing some testing on it now).
A followup to my own mail, after some quick testing:
My approach doesn't work (at least with microsoft web-folders). This is
because it applies to all the properties, and some of these are
apparently treated specially. Having the resourcetype property not be
<collection/> (exactly. Putting it in a CDATA section is too big a
change, apparently) for a directory seems to break MS web-folders.
So I see that the problem is a bit bigger. Your way works, but seems
rather like an ugly hack to me. If we can come up with a better
solution, that'd be good. That would probably end up having to be a
special treatment of displayname (to put it in a CDATA section) in
PropFindMethod.java
For now, I'll leave things as-is, and leave the decision up to you.
Michael
Re: cvs commit: jakarta-slide/src/share/org/apache/slide/content
ContentImpl.java NodeRevisionDescriptor.java
Posted by Michael Smith <ms...@xn.com.au>.
> public final class NodeRevisionDescriptor implements Serializable, Cloneable {
>
> @@ -542,7 +542,7 @@
> * @param name New name
> */
> public void setName(String name) {
> - setProperty(NAME, name, true);
> + setProperty(NAME, "<![CDATA[" + name + "]]>", true);
> }
Remy,
Can you justify this change? I was just trying to fix a bug. It happens
to be fixed by this, but I was seeing it with already-existing resources
and wanted to fix it for those as well - plus I didn't see this change
until I'd found it and fixed it differently. This fix conflicts and
makes my fix fail (which isn't a problem in itself, since you've already
fixed it, but I don't think this makes sense).
I understand the neccesity to not just blindly dump out an arbitrary
displayname, but this seems the wrong place to put it. We need (for
webdav) to escape certain characters that have special meaning to xml. A
CDATA section is one way to do this. However, it's unneccesary (and
confusing) to put the actual XML content into the stores (which this,
indirectly, does). Instead, it'd make much more sense (to me, anyway) to
store the displayname in raw form in the stores (since we assume,
sensibly, that the stores can store arbitrary data), and then to
encapsulate this in a CDATA section on output as xml.
This would have the additional benefit of fixing problems with existing
resources.
The fix then becomes removing this change, and changing a pair of
writeText() calls to writeData(), in PropFindMethod.java (the logical
place - the slide core shouldn't know about the output format of the
data, and doesn't in other places). If you agree, I'll commit this
change (just doing some testing on it now).
Michael