Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃毀馃彈馃懛 Accept and deliver What's New information #580

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions nextcloudappstore/api/v1/release/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ def __init__(self) -> None:
self.info_schema = read_relative_file(__file__, 'info.xsd')
self.info_xslt = read_relative_file(__file__, 'info.xslt')
self.pre_info_xslt = read_relative_file(__file__, 'pre-info.xslt')
self.changes_schema = read_relative_file(__file__, 'changes.xsd')
self.pre_changes_xslt = read_relative_file(__file__, 'pre-changes.xslt')
self.db_schema = read_relative_file(__file__, 'database.xsd')
self.pre_db_xslt = read_relative_file(__file__, 'pre-database.xslt')
self.languages = settings.LANGUAGES
Expand Down
55 changes: 55 additions & 0 deletions nextcloudappstore/api/v1/release/changes.xsd
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?xml version="1.0"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
elementFormDefault="qualified" attributeFormDefault="unqualified">
<xs:element name="release">
<xs:complexType>
<xs:sequence>
<xs:element name="changelog" maxOccurs="1" minOccurs="1">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be optional and re-use the CHANGELOG.md in the app folder by default
.

<xs:complexType>
<xs:attribute name="href" type="xs:anyURI" use="required" />
</xs:complexType>
</xs:element>
<xs:element name="whatsNew" minOccurs="1" maxOccurs="unbounded">
<xs:complexType>
<xs:sequence>
<xs:element name="regular" type="whatsNewInfo" minOccurs="1" maxOccurs="1" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there are only admin changes? like "ui for configuring additional mount type"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should consider that. Might make sense for the server, too, as the maintenance releases often don't have much to brag about, but subtle bug fixes.

<xs:element name="admin" type="whatsNewInfo" minOccurs="0" maxOccurs="1" />
</xs:sequence>
<xs:attribute name="lang" type="language" use="required" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MorrisJobke how would this interact with translation? Not sure what your plans are currently for translating xml contents.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</xs:complexType>
</xs:element>
</xs:sequence>
<xs:attribute name="version" type="version" use="required" />
</xs:complexType>
</xs:element>

<xs:simpleType name="version">
<xs:restriction base="xs:string">
<xs:pattern value="(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)"/>
<xs:minLength value="6"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop the min and max length requirements. min length of 6 makes 0.0.1 invalid btw.

<xs:maxLength value="12"/>
</xs:restriction>
</xs:simpleType>

<!-- based on xs:language, just uses _ as a separator between lang and region. -->
<xs:simpleType name="language">
<xs:restriction base="xs:token">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<xs:pattern
value="([a-zA-Z]{2}|[iI]-[a-zA-Z]+|[xX]-[a-zA-Z]{1,8})(_[a-zA-Z]{1,8})*"
/>
</xs:restriction>
</xs:simpleType>

<xs:complexType name="whatsNewInfo">
<xs:sequence>
<xs:element name="item" minOccurs="1" maxOccurs="3">
<xs:simpleType>
<xs:restriction base="xs:string">
<xs:minLength value="1"/>
</xs:restriction>
</xs:simpleType>
</xs:element>
</xs:sequence>
</xs:complexType>

</xs:schema>
33 changes: 27 additions & 6 deletions nextcloudappstore/api/v1/release/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,24 +196,24 @@ def create_safe_xml_parser() -> lxml.etree.XMLParser:
remove_blank_text=True, dtd_validation=False # type: ignore
) # type: ignore


def parse_app_metadata(xml: str, schema: str, pre_xslt: str,
xslt: str) -> Dict:
def parse_app_xml_file(xml: str, schema: str, pre_xslt: str,
xslt: str, filename: str) -> Dict:
"""
Parses, validates and maps the xml onto a dict
:argument xml the info.xml string to parse
:argument xml the xml string to parse
:argument schema the schema xml as string
:argument pre_xslt xslt which is run before validation to ensure that
everything is in the correct order and that unknown elements are excluded
:argument xslt the xslt to transform it to a matching structure
:argument filename the file that is parsed essentially for error output
:raises InvalidAppMetadataXmlException if the schema does not validate
:return the parsed xml as dict
"""
parser = create_safe_xml_parser()
try:
doc = lxml.etree.fromstring(bytes(xml, encoding='utf-8'), parser)
except lxml.etree.XMLSyntaxError as e:
msg = 'info.xml contains malformed xml: %s' % e
msg = '%s contains malformed xml: %s' % (filename, e)
raise XMLSyntaxError(msg)
for _ in doc.iter(lxml.etree.Entity): # type: ignore
raise InvalidAppMetadataXmlException('Must not contain entities')
Expand All @@ -224,15 +224,36 @@ def parse_app_metadata(xml: str, schema: str, pre_xslt: str,
try:
schema.assertValid(pre_transformed_doc) # type: ignore
except lxml.etree.DocumentInvalid as e:
msg = 'info.xml did not validate: %s' % e
msg = '%s did not validate: %s' % (filename, e)
raise InvalidAppMetadataXmlException(msg)
transform = lxml.etree.XSLT(lxml.etree.XML(xslt)) # type: ignore
transformed_doc = transform(pre_transformed_doc) # type: ignore
mapped = element_to_dict(transformed_doc.getroot()) # type: ignore
return mapped

def parse_app_metadata(xml: str, schema: str, pre_xslt: str,
xslt: str) -> Dict:
"""
Parses, validates and maps the xml onto a dict
:argument xml the info.xml string to parse
:argument schema the schema xml as string
:argument pre_xslt xslt which is run before validation to ensure that
everything is in the correct order and that unknown elements are excluded
:argument xslt the xslt to transform it to a matching structure
:raises InvalidAppMetadataXmlException if the schema does not validate
:return the parsed xml as dict
"""
mapped = parse_app_xml_file(xml, schema, pre_xslt, xslt, "info.xml")
validate_english_present(mapped)
fix_partial_translations(mapped)
return mapped

def parse_app_whats_new(xml: str, schema: str, pre_xslt: str,
xslt: str) -> Dict:
mapped = parse_app_xml_file(xml, schema, pre_xslt, xslt, "changes.xml")
validate_english_present(mapped)
fix_partial_translations(mapped)
return mapped

def validate_database(xml: str, schema: str, pre_xslt: str) -> None:
"""
Expand Down
37 changes: 37 additions & 0 deletions nextcloudappstore/api/v1/release/pre-changes.xslt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?xml version="1.0"?>
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:template match="/release">
<release>
<xsl:variable name="lowercase" select="'abcdefghijklmnopqrstuvwxyz'" />
<xsl:variable name="uppercase" select="'ABCDEFGHIJKLMNOPQRSTUVWXYZ'" />

<!-- reformat info.xml to have everything in order and excluded unknown elements -->
<xsl:copy-of select="changelog"/>
<xsl:apply-templates select="whatsNew"/>
</release>
</xsl:template>

<xsl:template match="whatsNew">
<whatsNew>
<xsl:if test="@lang">
<xsl:attribute name="lang">
<xsl:value-of select="@lang"/>
</xsl:attribute>
</xsl:if>
<xsl:apply-templates select="regular"/>
<xsl:apply-templates select="admin"/>
</whatsNew>
</xsl:template>

<xsl:template match="regular">
<regular>
<xsl:copy-of select="item"/>
</regular>
</xsl:template>

<xsl:template match="admin">
<admin>
<xsl:copy-of select="item"/>
</admin>
</xsl:template>
</xsl:stylesheet>
7 changes: 5 additions & 2 deletions nextcloudappstore/api/v1/release/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
from nextcloudappstore.api.v1.release.downloader import \
AppReleaseDownloader
from nextcloudappstore.api.v1.release.parser import \
GunZipAppMetadataExtractor, parse_app_metadata, parse_changelog, \
validate_database
GunZipAppMetadataExtractor, parse_app_metadata, parse_app_whats_new, \
parse_changelog, validate_database


class InvalidAppDirectoryException(ValidationError):
Expand Down Expand Up @@ -35,6 +35,9 @@ def get_release_info(self, url: str, is_nightly: bool = False) -> Release:
info = parse_app_metadata(meta.info_xml, self.config.info_schema,
self.config.pre_info_xslt,
self.config.info_xslt)
if meta.changes_xml:
whats_new = parse_app_whats_new(meta.changes_xml, self.config.changes_schema,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an unused variable. If it's just for validation simply remove the assignment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still WIP

self.config.pre_changes_xslt)
if meta.database_xml:
validate_database(meta.database_xml, self.config.db_schema,
self.config.pre_db_xslt)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="utf-8"?>
<release xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://apps.nextcloud.com/schema/apps/changes.xsd" version="0.0.1">
<changelog href="" />
<whatsNew lang="en">
<regular>
<item>First {{ app.name }} release!</item>
</regular>
<admin>
<item>requires {{ app.nextcloud_version }}</item>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is redundant with the version in info.xml and probably a bad example for new developers. Better example or simply remove it?

</admin>
</whatsNew>
</release>
1 change: 1 addition & 0 deletions nextcloudappstore/scaffolding/app-templates/15