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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing instance_id on GET request via preprocessor does not work #365

Closed
h0st1le opened this Issue Oct 17, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@h0st1le

h0st1le commented Oct 17, 2014

The documentation states: ' Preprocessors and postprocessors modify their arguments in-place.'

However, if I attempt to change the instance_id on a GET request it does not work. Although it appears to change the argument in-place, it does not return the changed id. Instead the original id requested, is used and the preprocessor modification is ignored.

I modified quickstart.py to give a working example below:

import os
import flask
import flask.ext.sqlalchemy
import flask.ext.restless

from datetime import datetime

_basedir = os.path.dirname(os.path.abspath(__file__))

# Create the Flask application and the Flask-SQLAlchemy object.
app = flask.Flask(__name__)
app.config['DEBUG'] = True
app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///' + _basedir + '/test.sqlite'
db = flask.ext.sqlalchemy.SQLAlchemy(app)

# Create your Flask-SQLALchemy models as usual but with the following two
# (reasonable) restrictions:
#   1. They must have a primary key column of type sqlalchemy.Integer or
#      type sqlalchemy.Unicode.
#   2. They must have an __init__ method which accepts keyword arguments for
#      all columns (the constructor in flask.ext.sqlalchemy.SQLAlchemy.Model
#      supplies such a method, so you don't need to declare a new one).


class Person(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.Unicode)
    birth_date = db.Column(db.Date)


class Computer(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.Unicode)
    vendor = db.Column(db.Unicode)
    purchase_time = db.Column(db.DateTime)
    owner_id = db.Column(db.Integer, db.ForeignKey('person.id'))
    owner = db.relationship('Person', backref=db.backref('computers',
                                                         lazy='dynamic'))


# Create the database tables.
db.create_all()

p1 = Person(name=u'Bob', birth_date=datetime.strptime('1980-12-14T05:00:00.000Z', '%Y-%m-%dT%H:%M:%S.%fZ'))
db.session.add(p1)

p2 = Person(name=u'Jim', birth_date=datetime.strptime('1982-08-22T05:00:00.000Z', '%Y-%m-%dT%H:%M:%S.%fZ'))
db.session.add(p2)

c1 = Computer(name=u'Bobs Computer', vendor=u'ASUS', purchase_time=datetime.strptime('2013-10-15T05:00:00.000Z', '%Y-%m-%dT%H:%M:%S.%fZ'), owner=p1)
db.session.add(c1)

c2 = Computer(name=u'Jims Computer', vendor=u'Apple', purchase_time=datetime.strptime('2013-08-22T05:00:00.000Z', '%Y-%m-%dT%H:%M:%S.%fZ'), owner=p2)
db.session.add(c2)
db.session.commit()

# Create the Flask-Restless API manager.
manager = flask.ext.restless.APIManager(app, flask_sqlalchemy_db=db)

def pre_get_single(instance_id=None, **kw):
    if instance_id == u'1':
        # "change instance_id from one to two"
        instance_id = u'2'
    else:
        # "change instance_id from two to one"
        instance_id = u'1'
    print instance_id

# Create API endpoints, which will be available at /api/<tablename> by
# default. Allowed HTTP methods can be specified as well.
manager.create_api(Person,
        methods=['GET', 'POST', 'DELETE'],
        preprocessors={
            'GET_SINGLE': [pre_get_single]
            }
        )
manager.create_api(Computer, methods=['GET'])

# start the flask loop
app.run()
@jfinkels

This comment has been minimized.

Owner

jfinkels commented Oct 21, 2014

There are really two issues here.

  1. This is a documentation bug. The instance_id parameter is going to be a "primitive" data type (like a string or an integer) most of the time, so assignments made within the preprocessor will not affect the value of the variable passed in by the calling function. What I should have written is "any modifications to dictionaries provided as parameters to preprocessor and postprocessor functions will be seen by the calling function, that is, the appropriate Flask-Restless view function." This should be fixed.
  2. This is a feature request. Flask-Restless doesn't currently allow you to modify the primary key provided to the view function. Is this a feature that would be useful and not too dangerous?

jfinkels added a commit that referenced this issue Oct 21, 2014

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Oct 21, 2014

Ah, even if we allowed a preprocessor to modify the primary key before Flask-Restless's main processing (in the API.get() method), what if there are multiple preprocessors? Any later one would override any assignment performed by an earlier one.

@h0st1le

This comment has been minimized.

h0st1le commented Oct 24, 2014

I don't actually know how 'dangerous' it would be to implement this type of behavior, nor how useful it would be for others. I happened to have a table that used a variety of look up codes to get the same object. So the input instance_id maybe something like 'px34' or 'ma56' which in turn I used to do a look up to find the actual instance_id of the table row '155' I then wanted to replace the original input instance_id with the proper table look up id in the preprocessor.

I'm not sure what would be different in this case regarding multiple preprocessors. Wouldn't multiple preprocessors override one another in the current form? I've not bothered to look into it, but perhaps their is a way we could specify the execution order of preprocessors?

jfinkels added a commit that referenced this issue Oct 25, 2014

jfinkels added a commit that referenced this issue Oct 25, 2014

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Oct 25, 2014

Yes, the preprocessors are each executed in the order specified by the user when passed as the preprocessors keyword argument to the create_api() method, so the last function to return a value would be interpreted as the new instance ID.

Meanwhile, I have created a branch at https://github.com/jfinkels/flask-restless/compare/processor-return that implements the requested functionality. Let me think it over and decide whether there's any problem with adding this feature.

@h0st1le

This comment has been minimized.

h0st1le commented Oct 28, 2014

Nice, I will check out the branch. thanks

jfinkels added a commit that referenced this issue Feb 2, 2015

@jfinkels

This comment has been minimized.

Owner

jfinkels commented Feb 3, 2015

The requested functionality will appear in version 0.16.0. See pull request #390.

@jfinkels jfinkels closed this Feb 3, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment