Edgewall Software
Modify

Opened 8 years ago

Last modified 2 years ago

#3332 assigned enhancement

Mimeview API v2

Reported by: cboos Owned by: cboos
Priority: high Milestone: next-major-releases
Component: rendering Version: devel
Severity: critical Keywords: mimeview
Cc: daved@…, ybarthelemy@…, olemis+trac@…, leho@…, ryan.j.ollos@…
Release Notes:
API Changes:

Description

We recently introduced the IContentConverter interface, and there are some issues remaining with this:

  • the methods for that interface are quite complex, not really extensible
  • the usage of strings for model objects should be replace by using the corresponding classes
  • artificial differences from the IHTMLPreviewRenderer, and duplicated code (mainly between Mimeview.render and Mimeview.convert_content)

Besides, there are other issues with the Mimetype API:

  • content_to_unicode needs to be deprecated/simplified as well
  • simplify the handling of mimetypes strings (the fact that they sometimes have the charset info, sometimes not)
  • in general, aim to simplify the code that uses the mimeview API

Work in progress. A patch will be attached today or tomorrow for review.

Attachments (4)

mimeview_refactoring-r3507.diff (82.1 KB) - added by cboos 8 years ago.
Initial steps of the refactoring. The new mimeview.api is in place. The Wiki, Attachment and Browser modules are using it. The individual IHTMLPreviewRenderer are still there, but used through a wrapper.
mimeview_refactoring-typerepr-r3507.diff (97.2 KB) - added by cboos 8 years ago.
Further progress on the refactoring. Now covers the Ticket module. For this, I had to introduce a base class for the MimeType?: it's the very general TypeRepr?, which is also subclassed by TypeWrapper?. This enable to use the IContentConverter machinery for converting plain old Python objects to other contents. Still need some more polishing, of course.
mimeview_api_refactoring-r3612.diff (117.3 KB) - added by cboos 8 years ago.
First stable snapshot of the refactoring
mimeview_api_refactoring-r3612.2.diff (119.1 KB) - added by cboos 8 years ago.
Slightly improved version

Download all attachments as: .zip

Change History (24)

Changed 8 years ago by cboos

Initial steps of the refactoring. The new mimeview.api is in place. The Wiki, Attachment and Browser modules are using it. The individual IHTMLPreviewRenderer are still there, but used through a wrapper.

comment:1 Changed 8 years ago by cboos

  • Owner changed from jonas to cboos
  • Status changed from new to assigned
  • Version changed from 0.9.5 to devel

The main idea of the refactoring is introduce the following new classes:

  • MimeType, whose main purpose is to simplify the code dealing with mimetype strings, by having always a clear separation between the mimetype itself and the optional charset information.
  • MimeContentBase classes, which abstract access to typed content. By default, there's a MimeContent class for encapsulating access to string content and a FileMimeContent class which encapsulates access to file content. The versioncontrol subsystem brings its own NodeMimeContent.
  • Conversion objects, for specifying a conversion.

The highlights of the mimeview API are:

  1. The new IContentConverter interface
    class IContentConverter(Interface):
        """An extension point interface for generic content conversion."""
    
        def get_supported_conversions(mimetype): 
            """Check if conversion of `mimetype` is supported by this converter.
    
            Return an iterable of `Conversion` objects for which this is
            the case.
            """
    
        def convert_content(context, conversion, content): 
            """Convert the given `content` using the specified `conversion`.
    
            The conversion takes place in the given formatting `context`.
    
            A `context` can provide at least a `req` property.
            
            Return the converted content as a new `MimeContent` object.
            """ 
    
  1. The new Mimeview methods, for rendering to HTML and for generic conversions:
    # -- MIME type conversion
    
    def get_conversions(self, input):
        """Return a list of possible conversions for the `input` MimeType.

        The returned list contains pair of `(conversion, converter)` objects,
        ordered from best to worst quality.
        """

    def convert(self, context, selector, mimecontent):
        """Convert the `mimecontent` to another MIME type.

        The conversion to be done is determined by `selector`,
        which can be either directly the desired output MIME type or
        a key identifying the `Conversion` object.

        Returns a new `MimeContent`.
        """

    def render(self, req, mimecontent, annotations=None):
        """Render an XHTML preview of the given `mimecontent`.

        Some `annotations` might be requested as well.
        """

    # -- Utilities

    def preview_to_hdf(self, req, mimecontent, annotations=None):
        """Prepares a rendered preview of the given `mimecontent`."""        

    def send_converted(self, req, selector, mimecontent):
        """Helper method for converting `mimecontent` and sending it directly.

        `selector` can be either a key or the expected output MIME Type.
        """

There are still some things in flux at this point, but I think it's now in a good state to get some early feedback…

Changed 8 years ago by cboos

Further progress on the refactoring. Now covers the Ticket module. For this, I had to introduce a base class for the MimeType?: it's the very general TypeRepr?, which is also subclassed by TypeWrapper?. This enable to use the IContentConverter machinery for converting plain old Python objects to other contents. Still need some more polishing, of course.

comment:2 Changed 8 years ago by cboos

  • Keywords review added

I think I have now a quite satisfying refactoring, which is a big leap ahead from the previous attempts.

The mimeview.api is now a bit more complex, but the IContentConverter is now simpler and more powerful, and all the code dealing with contents in the modules is now drastically simplified. The IHTMLPreviewRenderer interface is deprecated and components implementing it are now supported by an adapter implementing the new IContentConverter interface.

The functionality seems to be OK/improved for the following modules:

attachment, wiki, ticket, query, browser, changeset.

I append here the new mimeview.api module doc, which summarizes what's to be expected in the new API.


The trac.mimeview module centralizes the intelligence about typed content.

Originally, this was about file metadata, principally its MIME type and eventually the text encoding (charset) used by that content.

Now, this has evolved into managing any kind of typed content, and deals also with converting a content from one type to another. A common situation which is now handled part of the general case is the conversion of any kind of content to a text/html representation.

In order to keep the API of conversion interface IContentConverter simple, we introduced a few classes, each encapsulating a part of the knowledge related to the content and the conversion:

  • the ContentType is used to describe the type of a content. This is an abstract superclass for:
    • MimeType, used for storing the mime type string, the charset, and eventually the name and the commonly used file extension for that type
    • ObjectType, used when wrapping an arbitrary Python object
  • the AbstractContent, which wraps access to the actual content, and provides a few generic methods, among which convert() is the most important.
    • the ObjectContent, when the content is an arbitrary Python object
    • the MimeContent abstract class, which offers an uniform API to access the data over a wide range of containers:
      • the StringMimeContent, for handling string content (like str, unicode or Markup objects)
      • the StructuredMimeContent, for handling structured content (like Element objects)
      • the LineIteratorMimeContent, for handling line-oriented string content (like string iterators or string arrays)
      • the FileMimeContent, for handling file content
      • etc. (as an example, the repository layer defines a NodeMimeContent)
      That API provides different ways to get access to the data:
      • chunks(), an iterator for reading chunks of raw content
      • lines(), an iterator for reading lines from text content
      • markup(), when it makes sense to interpret the content as markup
      • __unicode()__, for converting the content to an unicode object
      • encode(), for converting the content to a str object. Note that we don't use __str__ for that on purpose, as we need to be able to specify the charset.
      • size(), for getting a hint about the content size without actually reading it.
  • the Conversion class, which is used to specify how a given IContentConverter component will perform a content conversion.

This corresponds to attachment:mimeview_api_refactoring-r3612.diff

Changed 8 years ago by cboos

First stable snapshot of the refactoring

comment:3 Changed 8 years ago by cboos

A few fixes and improvements:

  • Fix code block rendering in wiki (esp. when using the PlainTextRenderer?)
  • try/except protect the attempts to use the IHTMLPreviewRenderer
  • move getting the result.excerpt in the IHTMLPreviewRenderer adapter
  • Mimeview.render: should take the markup from MimeContent? results.
  • reST rendering now implements IContentConverter, and makes use of new annotable property.

This corresponds to attachment:mimeview_api_refactoring-r3612.diff.2

Changed 8 years ago by cboos

Slightly improved version

comment:4 Changed 8 years ago by cboos

  • Milestone changed from 0.10 to 0.10.1
  • Severity changed from blocker to major

This is post-poned to at least milestone:0.10.1

In the meantime, I'm still waiting for constructive criticisms ;)

comment:5 follow-up: Changed 8 years ago by tjb@…

I can't offer a criticism of the source itself, but I applied the patch, and our trac environment ceased to run after that. The apache log showed this…

[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend: Traceback (most recent call last):
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/mod_python/apache.py", line 287, in HandlerDispatch\n    log=debug)
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/mod_python/apache.py", line 457, in import_module\n    module = imp.load_module(mname, f, p, d)
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/trac/web/modpython_frontend.py", line 21, in ?\n    from trac.web.main import dispatch_request
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/trac/web/main.py", line 34, in ?\n    from trac.web.chrome import Chrome
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/trac/web/chrome.py", line 23, in ?\n    from trac.mimeview.api import get_mimetype, MimeContent
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/trac/mimeview/__init__.py", line 1, in ?\n    from trac.mimeview.api import *
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/trac/mimeview/api.py", line 669, in ?\n    class ImageRenderer(Component):
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend:   File "/usr/lib/python2.3/site-packages/trac/mimeview/api.py", line 677, in ImageRenderer\n    IMAGE_TYPE = MimeType(re.compile(r'image/'))
[Fri Sep 01 10:11:31 2006] [error] [client 172.16.2.115] PythonHandler trac.web.modpython_frontend: NameError: name 'MimeType' is not defined

The patch was applied against r3683. I reverted the patch out, and everyhing runs fine. As you can see, we are running with mod_python.

comment:6 in reply to: ↑ 5 Changed 8 years ago by cboos

Replying to tjb@cea.com.au:

from trac.mimeview.api import *
site-packages/trac/mimeview/api.py", line 677, in ImageRenderer\n IMAGE_TYPE = MimeType(re.compile(r'image/'))
NameError: name 'MimeType' is not defined

That's a bit weird though, as the MimeType class is defined in that file.

I can't reproduce the issue, though. Probably an installation issue, and the from trac.mimeview.api import * line picking up an unpatched trac.mimeview.api source.

comment:7 Changed 8 years ago by cboos

  • Keywords review removed

One thing that prevented progress of this ticket was that during testing in late August, I experienced strange application "freeze", triggered by concurrent requests. I was not able to figure out what was going on at that time, but I now strongly suspect that this was symptomatic of the #3504 issue, which is about to be fixed.

I'll update the patch.

comment:8 Changed 8 years ago by cboos

  • Milestone changed from 0.10.1 to 0.11

comment:9 Changed 8 years ago by anonymous

  • Cc daved@… added

comment:10 Changed 7 years ago by cboos

  • Milestone changed from 0.11 to 0.12

Not for 0.11.

comment:11 Changed 6 years ago by cboos

  • Severity changed from major to critical
  • Summary changed from Finalize the Mimeview API changes for 0.10 to Mimeview API v2
  • Type changed from task to enhancement

… so the patch didn't make it for 0.10 and also not for 0.11.

This has to be rewritten, and probably as an "api2.py", in order to avoid all the possible compatibility issues at once.

comment:12 Changed 6 years ago by ybarthelemy@…

  • Cc ybarthelemy@… added

comment:13 Changed 5 years ago by olemis+trac@…

  • Cc olemis+trac@… added

comment:14 Changed 4 years ago by cboos

  • Priority changed from high to normal

comment:15 follow-up: Changed 4 years ago by Carsten Klein <carsten.klein@…>

Alongside of this I would definitely put my vote in for externalizing the KNOWN_MIME_TYPES from trac to the operating system a/o the server handling/providing support for these mime types, so that a server administrator can keep all the information in a single location.

For a stand alone trac instance, this would require implementing platform specific mime type databases, wonder if python not already has an API for this somewhere…

comment:16 in reply to: ↑ 15 Changed 4 years ago by cboos

Replying to Carsten Klein <carsten.klein@…>:

Alongside of this I would definitely put my vote in for externalizing the KNOWN_MIME_TYPES from trac to the operating system a/o the server handling/providing support for these mime types, so that a server administrator can keep all the information in a single location.

For a stand alone trac instance, this would require implementing platform specific mime type databases, wonder if python not already has an API for this somewhere…

We already use the mimetypes module.

Note also that for customizations, the [mimeviewer] mime_map setting can be shared the normal way via [inherit] file.

comment:17 Changed 4 years ago by cboos

  • Component changed from general to rendering
  • Priority changed from normal to high

#9937 was requesting the ability to convert a milestone to other formats.

comment:18 Changed 2 years ago by lkraav <leho@…>

  • Cc leho@… added

comment:19 Changed 2 years ago by lkraav <leho@…>

I'm about to take a shot at writing a plugin to use pdf.js for rendering PDFs right on the preview page. Looks like this has stalled for the last two years, any updates or advice if I should worry myself with this for 1.0?

comment:20 Changed 2 years ago by Ryan J Ollos <ryan.j.ollos@…>

  • Cc ryan.j.ollos@… added

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned The owner will remain cboos.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from cboos to the specified user.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.