From 289fbd8640271f2c7558e09e6cec140b9f420f37 Mon Sep 17 00:00:00 2001 From: Chris Caron Date: Wed, 2 Sep 2020 17:42:17 -0400 Subject: [PATCH] Drop unnecessary file I/O handling + Auto-Detect Configuration Format (#18) --- README.md | 8 ++ apprise_api/api/forms.py | 4 + apprise_api/api/templates/config.html | 22 ++- apprise_api/api/tests/test_add.py | 118 +++++++++++++--- apprise_api/api/tests/test_json_urls.py | 19 --- apprise_api/api/tests/test_notify.py | 14 -- apprise_api/api/views.py | 172 +++++++++++------------- requirements.txt | 2 +- 8 files changed, 207 insertions(+), 152 deletions(-) diff --git a/README.md b/README.md index 46728a6..7bc54b2 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,10 @@ As an example, the `/json/urls/{KEY}` response might return something like this: Here is an example using `curl` as to how someone might send a notification to everyone associated with the tag `abc123` (using `/notify/{key}`): ```bash # Send notification(s) to a {key} defined as 'abc123' +curl -X POST -d "body=test message" \ + http://localhost:8000/notify/abc123 + +# Here is the same request but using JSON instead: curl -X POST -d '{"body":"test message"}' \ -H "Content-Type: application/json" \ http://localhost:8000/notify/abc123 @@ -118,6 +122,10 @@ curl -X POST -d '{"body":"test message"}' \ ```bash # Send notification(s) to a {key} defined as 'abc123' # but only notify the URLs associated with the 'devops' tag +curl -X POST -d 'tag=devops&body=test message' \ + http://localhost:8000/notify/abc123 + +# Here is the same request but using JSON instead: curl -X POST -d '{"tag":"devops", "body":"test message"}' \ -H "Content-Type: application/json" \ http://localhost:8000/notify/abc123 diff --git a/apprise_api/api/forms.py b/apprise_api/api/forms.py index 4fbb359..d316e40 100644 --- a/apprise_api/api/forms.py +++ b/apprise_api/api/forms.py @@ -27,8 +27,12 @@ import apprise from django import forms from django.utils.translation import gettext_lazy as _ +# Auto-Detect Keyword +AUTO_DETECT_CONFIG_KEYWORD = 'auto' + # Define our potential configuration types CONFIG_FORMATS = ( + (AUTO_DETECT_CONFIG_KEYWORD, _('Auto-Detect')), (apprise.ConfigFormat.TEXT, _('TEXT')), (apprise.ConfigFormat.YAML, _('YAML')), ) diff --git a/apprise_api/api/templates/config.html b/apprise_api/api/templates/config.html index 88611d0..7486d17 100644 --- a/apprise_api/api/templates/config.html +++ b/apprise_api/api/templates/config.html @@ -117,9 +117,11 @@ async function update() { // perform our status check let response = await fetch('{% url "get" key %}', { method: 'POST', + headers: { + 'Content-Type': 'application/json;charset=utf-8' + }, }); - let result = await response; if(response.status == 204) { // no problem; we simply have no content to retrieve @@ -133,10 +135,22 @@ async function update() { document.querySelector('.config-overview li a[href="#notify"]') .parentNode.classList.remove('disabled'); + // get our results + let result = await response.json(); + // Set our configuration so it's visible - response.text().then(function (text) { - document.querySelector('#id_config').value = text; - }); + document.querySelector('#id_config').value = result.config; + // Set our format + document.querySelector('#id_format').value = result.format; + + // dispatch our event to update our select box + if (typeof(Event) === 'function') { + var event = new Event('change'); + } else { // for IE11 + var event = document.createEvent('Event'); + event.initEvent('change', true, true); + } + document.querySelector('#id_format').dispatchEvent(event); // Ensure has-config sections are visible document.querySelector('.has-config') diff --git a/apprise_api/api/tests/test_add.py b/apprise_api/api/tests/test_add.py index 3dffe68..2e79109 100644 --- a/apprise_api/api/tests/test_add.py +++ b/apprise_api/api/tests/test_add.py @@ -25,6 +25,7 @@ from django.test import SimpleTestCase from apprise import ConfigFormat from unittest.mock import patch +from ..forms import AUTO_DETECT_CONFIG_KEYWORD import json @@ -129,7 +130,7 @@ class AddTests(SimpleTestCase): # Empty Text Configuration config = """ - + """ # noqa W293 response = self.client.post( '/add/{}'.format(key), { @@ -138,7 +139,7 @@ class AddTests(SimpleTestCase): # Valid Text Configuration config = """ - browser,media=notica://VToken + browser,media=notica://VTokenC home=mailto://user:pass@hotmail.com """ response = self.client.post( @@ -154,6 +155,27 @@ class AddTests(SimpleTestCase): ) assert response.status_code == 200 + # Valid Yaml Configuration + config = """ + urls: + - notica://VTokenD: + tag: browser,media + - mailto://user:pass@hotmail.com: + tag: home + """ + response = self.client.post( + '/add/{}'.format(key), + {'format': ConfigFormat.YAML, 'config': config}) + assert response.status_code == 200 + + # Test with JSON + response = self.client.post( + '/add/{}'.format(key), + data=json.dumps({'format': ConfigFormat.YAML, 'config': config}), + content_type='application/json', + ) + assert response.status_code == 200 + # Test invalid config format response = self.client.post( '/add/{}'.format(key), @@ -162,20 +184,6 @@ class AddTests(SimpleTestCase): ) assert response.status_code == 400 - with patch('tempfile.NamedTemporaryFile') as mock_ntf: - mock_ntf.side_effect = OSError - # we won't be able to write our retrieved configuration - # to disk for processing; we'll get a 500 error - response = self.client.post( - '/add/{}'.format(key), - data=json.dumps( - {'format': ConfigFormat.TEXT, 'config': config}), - content_type='application/json', - ) - - # internal errors are correctly identified - assert response.status_code == 500 - # Test the handling of underlining disk/write exceptions with patch('gzip.open') as mock_open: mock_open.side_effect = OSError() @@ -183,13 +191,89 @@ class AddTests(SimpleTestCase): response = self.client.post( '/add/{}'.format(key), data=json.dumps( - {'format': ConfigFormat.TEXT, 'config': config}), + {'format': ConfigFormat.YAML, 'config': config}), content_type='application/json', ) # internal errors are correctly identified assert response.status_code == 500 + def test_save_auto_detect_config_format(self): + """ + Test adding an configuration and using the autodetect feature + """ + + # our key to use + key = 'test_save_auto_detect_config_format' + + # Empty Text Configuration + config = """ + + """ # noqa W293 + response = self.client.post( + '/add/{}'.format(key), { + 'format': AUTO_DETECT_CONFIG_KEYWORD, 'config': config}) + assert response.status_code == 400 + + # Valid Text Configuration + config = """ + browser,media=notica://VTokenA + home=mailto://user:pass@hotmail.com + """ + response = self.client.post( + '/add/{}'.format(key), + {'format': AUTO_DETECT_CONFIG_KEYWORD, 'config': config}) + assert response.status_code == 200 + + # Test with JSON + response = self.client.post( + '/add/{}'.format(key), + data=json.dumps({'format': ConfigFormat.TEXT, 'config': config}), + content_type='application/json', + ) + assert response.status_code == 200 + + # Valid Yaml Configuration + config = """ + urls: + - notica://VTokenB: + tag: browser,media + + - mailto://user:pass@hotmail.com: + tag: home + """ + response = self.client.post( + '/add/{}'.format(key), + {'format': AUTO_DETECT_CONFIG_KEYWORD, 'config': config}) + assert response.status_code == 200 + + # Test with JSON + response = self.client.post( + '/add/{}'.format(key), + data=json.dumps( + {'format': AUTO_DETECT_CONFIG_KEYWORD, 'config': config}), + content_type='application/json', + ) + assert response.status_code == 200 + + # Test invalid config format that can not be auto-detected + config = """ + 42 + """ + response = self.client.post( + '/add/{}'.format(key), + {'format': AUTO_DETECT_CONFIG_KEYWORD, 'config': config}) + assert response.status_code == 400 + + # Test with JSON + response = self.client.post( + '/add/{}'.format(key), + data=json.dumps( + {'format': AUTO_DETECT_CONFIG_KEYWORD, 'config': config}), + content_type='application/json', + ) + assert response.status_code == 400 + def test_save_with_bad_input(self): """ Test adding with bad input in general diff --git a/apprise_api/api/tests/test_json_urls.py b/apprise_api/api/tests/test_json_urls.py index 5d27498..3ddf39a 100644 --- a/apprise_api/api/tests/test_json_urls.py +++ b/apprise_api/api/tests/test_json_urls.py @@ -136,25 +136,6 @@ class JsonUrlsTests(SimpleTestCase): assert 'tags' in response.json()['urls'][0] assert len(response.json()['urls'][0]['tags']) == 2 - # Handle case when we try to retrieve our content but we have no idea - # what the format is in. Essentialy there had to have been disk - # corruption here or someone meddling with the backend. - with patch('tempfile.NamedTemporaryFile') as mock_ntf: - mock_ntf.side_effect = OSError - # Now retrieve our JSON resonse - response = self.client.get('/json/urls/{}'.format(key)) - assert response.status_code == 500 - assert response['Content-Type'].startswith('application/json') - assert 'tags' in response.json() - assert 'urls' in response.json() - - # has error directive - assert 'error' in response.json() - - # entries exist by are empty - assert len(response.json()['tags']) == 0 - assert len(response.json()['urls']) == 0 - # Verify that the correct Content-Type is set in the header of the # response assert 'Content-Type' in response diff --git a/apprise_api/api/tests/test_notify.py b/apprise_api/api/tests/test_notify.py index 8119b88..ead1f44 100644 --- a/apprise_api/api/tests/test_notify.py +++ b/apprise_api/api/tests/test_notify.py @@ -212,20 +212,6 @@ class NotifyTests(SimpleTestCase): 'body': 'test message' } - with patch('tempfile.NamedTemporaryFile') as mock_ntf: - mock_ntf.side_effect = OSError - # we won't be able to write our retrieved configuration - # to disk for processing; we'll get a 500 error - response = self.client.post( - '/notify/{}'.format(key), - data=json.dumps(json_data), - content_type='application/json', - ) - - # internal errors are correctly identified - assert response.status_code == 500 - assert mock_notify.call_count == 0 - # Test the handling of underlining disk/write exceptions with patch('gzip.open') as mock_open: mock_open.side_effect = OSError() diff --git a/apprise_api/api/views.py b/apprise_api/api/views.py index 6d2d290..5eb5b68 100644 --- a/apprise_api/api/views.py +++ b/apprise_api/api/views.py @@ -38,8 +38,9 @@ from .forms import AddByUrlForm from .forms import AddByConfigForm from .forms import NotifyForm from .forms import NotifyByUrlForm +from .forms import CONFIG_FORMATS +from .forms import AUTO_DETECT_CONFIG_KEYWORD -import tempfile import apprise import json import re @@ -175,7 +176,7 @@ class AddView(View): elif 'config' in content: fmt = content.get('format', '').lower() - if fmt not in apprise.CONFIG_FORMATS: + if fmt not in [i[0] for i in CONFIG_FORMATS]: # Format must be one supported by apprise return HttpResponse( _('The format specified is invalid.'), @@ -185,41 +186,32 @@ class AddView(View): # prepare our apprise config object ac_obj = apprise.AppriseConfig() - try: - # Write our file to a temporary file - with tempfile.NamedTemporaryFile() as f: - # Write our content to disk - f.write(content['config'].encode()) - f.flush() + if fmt == AUTO_DETECT_CONFIG_KEYWORD: + # By setting format to None, it is automatically detected from + # within the add_config() call + fmt = None - if not ac_obj.add( - 'file://{}?format={}'.format(f.name, fmt)): - - # Bad Configuration - return HttpResponse( - _('The configuration specified is invalid.'), - status=ResponseCode.bad_request, - ) - - # Add our configuration - a_obj.add(ac_obj) - - if not len(a_obj): - # No specified URL(s) were loaded due to - # mis-configuration on the caller's part - return HttpResponse( - _('No valid URL(s) were specified.'), - status=ResponseCode.bad_request, - ) - - except OSError: - # We could not write the temporary file to disk + # Load our configuration + if not ac_obj.add_config(content['config'], format=fmt): + # The format could not be detected return HttpResponse( - _('The configuration could not be loaded.'), - status=ResponseCode.internal_server_error, + _('The configuration format could not be detected.'), + status=ResponseCode.bad_request, ) - if not ConfigCache.put(key, content['config'], fmt=fmt): + # Add our configuration + a_obj.add(ac_obj) + + if not len(a_obj): + # No specified URL(s) were loaded due to + # mis-configuration on the caller's part + return HttpResponse( + _('No valid URL(s) were specified.'), + status=ResponseCode.bad_request, + ) + + if not ConfigCache.put( + key, content['config'], fmt=ac_obj[0].config_format): # Something went very wrong; return 500 return HttpResponse( _('An error occured saving configuration.'), @@ -281,6 +273,9 @@ class GetView(View): Handle a POST request """ + # Detect the format our response should be in + json_response = MIME_IS_JSON.match(request.content_type) is not None + config, format = ConfigCache.get(key) if config is None: # The returned value of config and format tell a rather cryptic @@ -294,12 +289,24 @@ class GetView(View): return HttpResponse( _('There was no configuration found.'), status=ResponseCode.no_content, + ) if not json_response else JsonResponse({ + 'error': _('There was no configuration found.') + }, + encoder=JSONEncoder, + safe=False, + status=ResponseCode.no_content, ) # Something went very wrong; return 500 return HttpResponse( _('An error occured accessing configuration.'), status=ResponseCode.internal_server_error, + ) if not json_response else JsonResponse({ + 'error': _('There was no configuration found.') + }, + encoder=JSONEncoder, + safe=False, + status=ResponseCode.internal_server_error, ) # Our configuration was retrieved; now our response varies on whether @@ -315,6 +322,13 @@ class GetView(View): config, content_type=content_type, status=ResponseCode.okay, + ) if not json_response else JsonResponse({ + 'format': format, + 'config': config, + }, + encoder=JSONEncoder, + safe=False, + status=ResponseCode.okay, ) @@ -391,42 +405,26 @@ class NotifyView(View): # Create an apprise config object ac_obj = apprise.AppriseConfig() - try: - # Write our file to a temporary file containing our configuration - # so that we can read it back. In the future a change will be to - # Apprise so that we can just directly write the configuration as - # is to the AppriseConfig() object... but for now... - with tempfile.NamedTemporaryFile() as f: - # Write our content to disk - f.write(config.encode()) - f.flush() + # Load our configuration + ac_obj.add_config(config, format=format) - # Read our configuration back in to our configuration - ac_obj.add('file://{}?format={}'.format(f.name, format)) + # Add our configuration + a_obj.add(ac_obj) - # Add our configuration - a_obj.add(ac_obj) + # Perform our notification at this point + result = a_obj.notify( + content.get('body'), + title=content.get('title', ''), + notify_type=content.get('type', apprise.NotifyType.INFO), + tag=content.get('tag'), + ) - # Perform our notification at this point - result = a_obj.notify( - content.get('body'), - title=content.get('title', ''), - notify_type=content.get('type', apprise.NotifyType.INFO), - tag=content.get('tag'), - ) - - if not result: - # If at least one notification couldn't be sent; change up - # the response to a 424 error code - return HttpResponse( - _('One or more notification could not be sent.'), - status=ResponseCode.failed_dependency) - - except OSError: - # We could not write the temporary file to disk + if not result: + # If at least one notification couldn't be sent; change up + # the response to a 424 error code return HttpResponse( - _('The configuration could not be loaded.'), - status=ResponseCode.internal_server_error) + _('One or more notification could not be sent.'), + status=ResponseCode.failed_dependency) # Return our retrieved content return HttpResponse( @@ -582,41 +580,21 @@ class JsonUrlView(View): # Create an apprise config object ac_obj = apprise.AppriseConfig() - try: - # Write our file to a temporary file containing our configuration - # so that we can read it back. In the future a change will be to - # Apprise so that we can just directly write the configuration as - # is to the AppriseConfig() object... but for now... - with tempfile.NamedTemporaryFile() as f: - # Write our content to disk - f.write(config.encode()) - f.flush() + # Load our configuration + ac_obj.add_config(config, format=format) - # Read our configuration back in to our configuration - ac_obj.add('file://{}?format={}'.format(f.name, format)) + # Add our configuration + a_obj.add(ac_obj) - # Add our configuration - a_obj.add(ac_obj) + for notification in a_obj: + # Set Notification + response['urls'].append({ + 'url': notification.url(privacy=privacy), + 'tags': notification.tags, + }) - for notification in a_obj: - # Set Notification - response['urls'].append({ - 'url': notification.url(privacy=privacy), - 'tags': notification.tags, - }) - - # Store Tags - response['tags'] |= notification.tags - - except OSError: - # We could not write the temporary file to disk - response['error'] = _('The configuration could not be loaded.'), - return JsonResponse( - response, - encoder=JSONEncoder, - safe=False, - status=ResponseCode.internal_server_error, - ) + # Store Tags + response['tags'] |= notification.tags # Return our retrieved content return JsonResponse( diff --git a/requirements.txt b/requirements.txt index bd22b70..d3cee8d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ django -apprise +apprise >= 0.8.8